| Message ID | 20221219154119.419176389@infradead.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | Introduce cmpxchg128() -- aka. the demise of cmpxchg_double() | expand |
On Mon, 2022-12-19 at 16:35 +0100, Peter Zijlstra wrote: > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > --- > drivers/iommu/amd/amd_iommu_types.h | 9 +++++++-- > drivers/iommu/amd/iommu.c | 10 ++++------ > 2 files changed, 11 insertions(+), 8 deletions(-) > > --- a/drivers/iommu/amd/amd_iommu_types.h > +++ b/drivers/iommu/amd/amd_iommu_types.h > @@ -979,8 +979,13 @@ union irte_ga_hi { > }; > > struct irte_ga { > - union irte_ga_lo lo; > - union irte_ga_hi hi; > + union { > + struct { > + union irte_ga_lo lo; > + union irte_ga_hi hi; > + }; > + u128 irte; > + }; > }; > > struct irq_2_irte { > --- a/drivers/iommu/amd/iommu.c > +++ b/drivers/iommu/amd/iommu.c > @@ -2992,10 +2992,10 @@ static int alloc_irq_index(struct amd_io > static int modify_irte_ga(struct amd_iommu *iommu, u16 devid, int index, > struct irte_ga *irte, struct amd_ir_data *data) > { > - bool ret; > struct irq_remap_table *table; > - unsigned long flags; > struct irte_ga *entry; > + unsigned long flags; > + u128 old; > > table = get_irq_table(iommu, devid); > if (!table) > @@ -3006,16 +3006,14 @@ static int modify_irte_ga(struct amd_iom > entry = (struct irte_ga *)table->table; > entry = &entry[index]; > > - ret = cmpxchg_double(&entry->lo.val, &entry->hi.val, > - entry->lo.val, entry->hi.val, > - irte->lo.val, irte->hi.val); > /* > * We use cmpxchg16 to atomically update the 128-bit IRTE, > * and it cannot be updated by the hardware or other processors > * behind us, so the return value of cmpxchg16 should be the > * same as the old value. The above comment seems to have already been out of date but could be updated to say cmpxchg128 instead of cmxchg16 anyway. > */ > - WARN_ON(!ret); > + old = entry->irte; > + WARN_ON(!try_cmpxchg128(&entry->irte, &old, irte->irte)); > > if (data) > data->ref = entry; > >
On 12/19/2022 9:05 PM, Peter Zijlstra wrote: > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > --- > drivers/iommu/amd/amd_iommu_types.h | 9 +++++++-- > drivers/iommu/amd/iommu.c | 10 ++++------ > 2 files changed, 11 insertions(+), 8 deletions(-) > > --- a/drivers/iommu/amd/amd_iommu_types.h > +++ b/drivers/iommu/amd/amd_iommu_types.h > @@ -979,8 +979,13 @@ union irte_ga_hi { > }; > > struct irte_ga { > - union irte_ga_lo lo; > - union irte_ga_hi hi; > + union { > + struct { > + union irte_ga_lo lo; > + union irte_ga_hi hi; > + }; > + u128 irte; > + }; > }; > > struct irq_2_irte { > --- a/drivers/iommu/amd/iommu.c > +++ b/drivers/iommu/amd/iommu.c > @@ -2992,10 +2992,10 @@ static int alloc_irq_index(struct amd_io > static int modify_irte_ga(struct amd_iommu *iommu, u16 devid, int index, > struct irte_ga *irte, struct amd_ir_data *data) > { > - bool ret; > struct irq_remap_table *table; > - unsigned long flags; > struct irte_ga *entry; > + unsigned long flags; > + u128 old; > > table = get_irq_table(iommu, devid); > if (!table) > @@ -3006,16 +3006,14 @@ static int modify_irte_ga(struct amd_iom > entry = (struct irte_ga *)table->table; > entry = &entry[index]; > > - ret = cmpxchg_double(&entry->lo.val, &entry->hi.val, > - entry->lo.val, entry->hi.val, > - irte->lo.val, irte->hi.val); > /* > * We use cmpxchg16 to atomically update the 128-bit IRTE, > * and it cannot be updated by the hardware or other processors > * behind us, so the return value of cmpxchg16 should be the > * same as the old value. > */ > - WARN_ON(!ret); > + old = entry->irte; > + WARN_ON(!try_cmpxchg128(&entry->irte, &old, irte->irte)); Changes looks good to me. I have tested it on AMD system and it works fine. -Vasant
--- a/drivers/iommu/amd/amd_iommu_types.h +++ b/drivers/iommu/amd/amd_iommu_types.h @@ -979,8 +979,13 @@ union irte_ga_hi { }; struct irte_ga { - union irte_ga_lo lo; - union irte_ga_hi hi; + union { + struct { + union irte_ga_lo lo; + union irte_ga_hi hi; + }; + u128 irte; + }; }; struct irq_2_irte { --- a/drivers/iommu/amd/iommu.c +++ b/drivers/iommu/amd/iommu.c @@ -2992,10 +2992,10 @@ static int alloc_irq_index(struct amd_io static int modify_irte_ga(struct amd_iommu *iommu, u16 devid, int index, struct irte_ga *irte, struct amd_ir_data *data) { - bool ret; struct irq_remap_table *table; - unsigned long flags; struct irte_ga *entry; + unsigned long flags; + u128 old; table = get_irq_table(iommu, devid); if (!table) @@ -3006,16 +3006,14 @@ static int modify_irte_ga(struct amd_iom entry = (struct irte_ga *)table->table; entry = &entry[index]; - ret = cmpxchg_double(&entry->lo.val, &entry->hi.val, - entry->lo.val, entry->hi.val, - irte->lo.val, irte->hi.val); /* * We use cmpxchg16 to atomically update the 128-bit IRTE, * and it cannot be updated by the hardware or other processors * behind us, so the return value of cmpxchg16 should be the * same as the old value. */ - WARN_ON(!ret); + old = entry->irte; + WARN_ON(!try_cmpxchg128(&entry->irte, &old, irte->irte)); if (data) data->ref = entry;
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> --- drivers/iommu/amd/amd_iommu_types.h | 9 +++++++-- drivers/iommu/amd/iommu.c | 10 ++++------ 2 files changed, 11 insertions(+), 8 deletions(-)