| Message ID | 5D024E3E0200007800237E03@prv1-mh.provo.novell.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86: AMD x2APIC support | expand |
On 13/06/2019 14:23, Jan Beulich wrote: > At the same time restrict its scope to just the single source file > actually using it, and abstract accesses by introducing a union of > pointers. (A union of the actual table entries is not used to make it > impossible to [wrongly, once the 128-bit form gets added] perform > pointer arithmetic / array accesses on derived types.) > > Also move away from updating the entries piecemeal: Construct a full new > entry, and write it out. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > --- > It would have been nice to use write_atomic() or ACCESS_ONCE() for the > actual writes, but both cast the value to a scalar one, which doesn't > suit us here (and I also didn't want to make the compound type a union > with a raw member just for this). > > --- a/xen/drivers/passthrough/amd/iommu_intr.c > +++ b/xen/drivers/passthrough/amd/iommu_intr.c > @@ -23,6 +23,23 @@ > #include <asm/io_apic.h> > #include <xen/keyhandler.h> > > +struct irte_basic { I'd suggest irte_32, to go with irte_128 in the following patch. The 128bit format is also used for posted interrupts, and isn't specific to x2apic support. Furthermore, calling it irte_full isn't a term I can see in the manual, and is falling into the naming trap that USB currently lives in. > + unsigned int remap_en:1; > + unsigned int sup_io_pf:1; > + unsigned int int_type:3; > + unsigned int rq_eoi:1; > + unsigned int dm:1; > + unsigned int guest_mode:1; /* MBZ */ > + unsigned int dest:8; > + unsigned int vector:8; > + unsigned int :8; > +}; > + > +union irte_ptr { > + void *raw; > + struct irte_basic *basic; > +}; > + > #define INTREMAP_TABLE_ORDER 1 > #define INTREMAP_LENGTH 0xB > #define INTREMAP_ENTRIES (1 << INTREMAP_LENGTH) > @@ -101,47 +118,44 @@ static unsigned int alloc_intremap_entry > return slot; > } > > -static u32 *get_intremap_entry(int seg, int bdf, int offset) > +static union irte_ptr get_intremap_entry(unsigned int seg, unsigned int bdf, > + unsigned int offset) As this is changing, s/offset/entry/ to avoid any confusion where offset might be in units of bytes. ~Andrew
On 13/06/2019 14:23, Jan Beulich wrote: > At the same time restrict its scope to just the single source file > actually using it, and abstract accesses by introducing a union of > pointers. (A union of the actual table entries is not used to make it > impossible to [wrongly, once the 128-bit form gets added] perform > pointer arithmetic / array accesses on derived types.) > > Also move away from updating the entries piecemeal: Construct a full new > entry, and write it out. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > --- > It would have been nice to use write_atomic() or ACCESS_ONCE() for the > actual writes, but both cast the value to a scalar one, which doesn't > suit us here (and I also didn't want to make the compound type a union > with a raw member just for this). Actually, having looked at the following patch, I think it would be better to union with a uint32_t raw, so that we can use... > @@ -101,47 +118,44 @@ static unsigned int alloc_intremap_entry > return slot; > } > > -static u32 *get_intremap_entry(int seg, int bdf, int offset) > +static union irte_ptr get_intremap_entry(unsigned int seg, unsigned int bdf, > + unsigned int offset) > { > - u32 *table = get_ivrs_mappings(seg)[bdf].intremap_table; > + union irte_ptr table = { > + .raw = get_ivrs_mappings(seg)[bdf].intremap_table > + }; > + > + ASSERT(table.raw && (offset < INTREMAP_ENTRIES)); > > - ASSERT( (table != NULL) && (offset < INTREMAP_ENTRIES) ); > + table.basic += offset; > > - return table + offset; > + return table; > } > > -static void free_intremap_entry(int seg, int bdf, int offset) > +static void free_intremap_entry(unsigned int seg, unsigned int bdf, unsigned int offset) > { > - u32 *entry = get_intremap_entry(seg, bdf, offset); > + union irte_ptr entry = get_intremap_entry(seg, bdf, offset); > + > + *entry.basic = (struct irte_basic){}; ACCESS_ONCE(*entry.basic.raw) = 0; and... > > - memset(entry, 0, sizeof(u32)); > __clear_bit(offset, get_ivrs_mappings(seg)[bdf].intremap_inuse); > } > > -static void update_intremap_entry(u32* entry, u8 vector, u8 int_type, > - u8 dest_mode, u8 dest) > -{ > - set_field_in_reg_u32(IOMMU_CONTROL_ENABLED, 0, > - INT_REMAP_ENTRY_REMAPEN_MASK, > - INT_REMAP_ENTRY_REMAPEN_SHIFT, entry); > - set_field_in_reg_u32(IOMMU_CONTROL_DISABLED, *entry, > - INT_REMAP_ENTRY_SUPIOPF_MASK, > - INT_REMAP_ENTRY_SUPIOPF_SHIFT, entry); > - set_field_in_reg_u32(int_type, *entry, > - INT_REMAP_ENTRY_INTTYPE_MASK, > - INT_REMAP_ENTRY_INTTYPE_SHIFT, entry); > - set_field_in_reg_u32(IOMMU_CONTROL_DISABLED, *entry, > - INT_REMAP_ENTRY_REQEOI_MASK, > - INT_REMAP_ENTRY_REQEOI_SHIFT, entry); > - set_field_in_reg_u32((u32)dest_mode, *entry, > - INT_REMAP_ENTRY_DM_MASK, > - INT_REMAP_ENTRY_DM_SHIFT, entry); > - set_field_in_reg_u32((u32)dest, *entry, > - INT_REMAP_ENTRY_DEST_MAST, > - INT_REMAP_ENTRY_DEST_SHIFT, entry); > - set_field_in_reg_u32((u32)vector, *entry, > - INT_REMAP_ENTRY_VECTOR_MASK, > - INT_REMAP_ENTRY_VECTOR_SHIFT, entry); > +static void update_intremap_entry(union irte_ptr entry, unsigned int vector, > + unsigned int int_type, > + unsigned int dest_mode, unsigned int dest) > +{ > + struct irte_basic basic = { > + .remap_en = 1, > + .sup_io_pf = 0, > + .int_type = int_type, > + .rq_eoi = 0, > + .dm = dest_mode, > + .dest = dest, > + .vector = vector, > + }; > + > + *entry.basic = basic; ACCESS_ONCE(*entry.basic.raw) = basic.raw. The problem is in an unoptimised case, structure assignment in implemented with memcpy(), which may be implemented as `rep stosb` which may result in a spliced write with the enable bit set first. Using a union with raw allows for the use of ACCESS_ONCE(), which forces the compiler to implement them as 32bit single mov's. ~Andrew
>>> On 18.06.19 at 13:31, <andrew.cooper3@citrix.com> wrote: > On 13/06/2019 14:23, Jan Beulich wrote: >> At the same time restrict its scope to just the single source file >> actually using it, and abstract accesses by introducing a union of >> pointers. (A union of the actual table entries is not used to make it >> impossible to [wrongly, once the 128-bit form gets added] perform >> pointer arithmetic / array accesses on derived types.) >> >> Also move away from updating the entries piecemeal: Construct a full new >> entry, and write it out. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> --- >> It would have been nice to use write_atomic() or ACCESS_ONCE() for the >> actual writes, but both cast the value to a scalar one, which doesn't >> suit us here (and I also didn't want to make the compound type a union >> with a raw member just for this). > > Actually, having looked at the following patch, I think it would be > better to union with a uint32_t raw, so that we can use... Well, I did in fact have it that way first, until ... >> +static void update_intremap_entry(union irte_ptr entry, unsigned int vector, >> + unsigned int int_type, >> + unsigned int dest_mode, unsigned int dest) >> +{ >> + struct irte_basic basic = { >> + .remap_en = 1, >> + .sup_io_pf = 0, >> + .int_type = int_type, >> + .rq_eoi = 0, >> + .dm = dest_mode, >> + .dest = dest, >> + .vector = vector, >> + }; ... I figured that this initializer then will require the bitfields part of the union to also get named, like for union amd_iommu_ext_features in patch 1. >> + *entry.basic = basic; > > ACCESS_ONCE(*entry.basic.raw) = basic.raw. > > The problem is in an unoptimised case, structure assignment in > implemented with memcpy(), which may be implemented as `rep stosb` which > may result in a spliced write with the enable bit set first. > > Using a union with raw allows for the use of ACCESS_ONCE(), which forces > the compiler to implement them as 32bit single mov's. If we are worried about this, writing of 32-bit entries could be done (as an alternative to what you suggest) just like that of 128-bit ones by the last patch in the series. Jan
>>> On 18.06.19 at 12:37, <andrew.cooper3@citrix.com> wrote: > On 13/06/2019 14:23, Jan Beulich wrote: >> At the same time restrict its scope to just the single source file >> actually using it, and abstract accesses by introducing a union of >> pointers. (A union of the actual table entries is not used to make it >> impossible to [wrongly, once the 128-bit form gets added] perform >> pointer arithmetic / array accesses on derived types.) >> >> Also move away from updating the entries piecemeal: Construct a full new >> entry, and write it out. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> --- >> It would have been nice to use write_atomic() or ACCESS_ONCE() for the >> actual writes, but both cast the value to a scalar one, which doesn't >> suit us here (and I also didn't want to make the compound type a union >> with a raw member just for this). >> >> --- a/xen/drivers/passthrough/amd/iommu_intr.c >> +++ b/xen/drivers/passthrough/amd/iommu_intr.c >> @@ -23,6 +23,23 @@ >> #include <asm/io_apic.h> >> #include <xen/keyhandler.h> >> >> +struct irte_basic { > > I'd suggest irte_32, to go with irte_128 in the following patch. > > The 128bit format is also used for posted interrupts, and isn't specific > to x2apic support. There are still two forms of 128-bit entries, and the intention with the chosen names was for the other one to become irte_guest. > Furthermore, calling it irte_full isn't a term I can see in the manual, > and is falling into the naming trap that USB currently lives in. Except that other than for USB's transfer speeds I can't really see this getting wider and wider. >> @@ -101,47 +118,44 @@ static unsigned int alloc_intremap_entry >> return slot; >> } >> >> -static u32 *get_intremap_entry(int seg, int bdf, int offset) >> +static union irte_ptr get_intremap_entry(unsigned int seg, unsigned int bdf, >> + unsigned int offset) > > As this is changing, s/offset/entry/ to avoid any confusion where offset > might be in units of bytes. I don't really mind - I think both names are sufficiently clear, but I'll switch since you think the other name is better. Jan
On 18/06/2019 12:53, Jan Beulich wrote: >>>> On 18.06.19 at 12:37, <andrew.cooper3@citrix.com> wrote: >> On 13/06/2019 14:23, Jan Beulich wrote: >>> At the same time restrict its scope to just the single source file >>> actually using it, and abstract accesses by introducing a union of >>> pointers. (A union of the actual table entries is not used to make it >>> impossible to [wrongly, once the 128-bit form gets added] perform >>> pointer arithmetic / array accesses on derived types.) >>> >>> Also move away from updating the entries piecemeal: Construct a full new >>> entry, and write it out. >>> >>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >>> --- >>> It would have been nice to use write_atomic() or ACCESS_ONCE() for the >>> actual writes, but both cast the value to a scalar one, which doesn't >>> suit us here (and I also didn't want to make the compound type a union >>> with a raw member just for this). >>> >>> --- a/xen/drivers/passthrough/amd/iommu_intr.c >>> +++ b/xen/drivers/passthrough/amd/iommu_intr.c >>> @@ -23,6 +23,23 @@ >>> #include <asm/io_apic.h> >>> #include <xen/keyhandler.h> >>> >>> +struct irte_basic { >> I'd suggest irte_32, to go with irte_128 in the following patch. >> >> The 128bit format is also used for posted interrupts, and isn't specific >> to x2apic support. > There are still two forms of 128-bit entries, and the intention with > the chosen names was for the other one to become irte_guest. They are not forms of which can be delineated by irte_mode, because the guest_mode setting is (/will be) per-domain, not global (which is necessary for sane testability, and for nested-virt support where the guest VMCB controls aren't set up by Xen). > >> Furthermore, calling it irte_full isn't a term I can see in the manual, >> and is falling into the naming trap that USB currently lives in. > Except that other than for USB's transfer speeds I can't really see > this getting wider and wider. It doesn't make the names "basic" and "full" any more descriptive. > >>> @@ -101,47 +118,44 @@ static unsigned int alloc_intremap_entry >>> return slot; >>> } >>> >>> -static u32 *get_intremap_entry(int seg, int bdf, int offset) >>> +static union irte_ptr get_intremap_entry(unsigned int seg, unsigned int bdf, >>> + unsigned int offset) >> As this is changing, s/offset/entry/ to avoid any confusion where offset >> might be in units of bytes. > I don't really mind - I think both names are sufficiently clear, but > I'll switch since you think the other name is better. Looking through the other code, idx or index would also do fine, but I think all of these are clearer than using offset. ~Andrew
>>> On 18.06.19 at 14:16, <andrew.cooper3@citrix.com> wrote: > On 18/06/2019 12:53, Jan Beulich wrote: >>>>> On 18.06.19 at 12:37, <andrew.cooper3@citrix.com> wrote: >>> On 13/06/2019 14:23, Jan Beulich wrote: >>>> --- a/xen/drivers/passthrough/amd/iommu_intr.c >>>> +++ b/xen/drivers/passthrough/amd/iommu_intr.c >>>> @@ -23,6 +23,23 @@ >>>> #include <asm/io_apic.h> >>>> #include <xen/keyhandler.h> >>>> >>>> +struct irte_basic { >>> I'd suggest irte_32, to go with irte_128 in the following patch. >>> >>> The 128bit format is also used for posted interrupts, and isn't specific >>> to x2apic support. >> There are still two forms of 128-bit entries, and the intention with >> the chosen names was for the other one to become irte_guest. > > They are not forms of which can be delineated by irte_mode, because the > guest_mode setting is (/will be) per-domain, not global (which is > necessary for sane testability, and for nested-virt support where the > guest VMCB controls aren't set up by Xen). True and ... >>> Furthermore, calling it irte_full isn't a term I can see in the manual, >>> and is falling into the naming trap that USB currently lives in. >> Except that other than for USB's transfer speeds I can't really see >> this getting wider and wider. > > It doesn't make the names "basic" and "full" any more descriptive. ... also true, but irte_128 still won't fly with the other (guest) layout. >>>> @@ -101,47 +118,44 @@ static unsigned int alloc_intremap_entry >>>> return slot; >>>> } >>>> >>>> -static u32 *get_intremap_entry(int seg, int bdf, int offset) >>>> +static union irte_ptr get_intremap_entry(unsigned int seg, unsigned int bdf, >>>> + unsigned int offset) >>> As this is changing, s/offset/entry/ to avoid any confusion where offset >>> might be in units of bytes. >> I don't really mind - I think both names are sufficiently clear, but >> I'll switch since you think the other name is better. > > Looking through the other code, idx or index would also do fine, but I > think all of these are clearer than using offset. "index" it is then. Jan
--- a/xen/drivers/passthrough/amd/iommu_intr.c +++ b/xen/drivers/passthrough/amd/iommu_intr.c @@ -23,6 +23,23 @@ #include <asm/io_apic.h> #include <xen/keyhandler.h> +struct irte_basic { + unsigned int remap_en:1; + unsigned int sup_io_pf:1; + unsigned int int_type:3; + unsigned int rq_eoi:1; + unsigned int dm:1; + unsigned int guest_mode:1; /* MBZ */ + unsigned int dest:8; + unsigned int vector:8; + unsigned int :8; +}; + +union irte_ptr { + void *raw; + struct irte_basic *basic; +}; + #define INTREMAP_TABLE_ORDER 1 #define INTREMAP_LENGTH 0xB #define INTREMAP_ENTRIES (1 << INTREMAP_LENGTH) @@ -101,47 +118,44 @@ static unsigned int alloc_intremap_entry return slot; } -static u32 *get_intremap_entry(int seg, int bdf, int offset) +static union irte_ptr get_intremap_entry(unsigned int seg, unsigned int bdf, + unsigned int offset) { - u32 *table = get_ivrs_mappings(seg)[bdf].intremap_table; + union irte_ptr table = { + .raw = get_ivrs_mappings(seg)[bdf].intremap_table + }; + + ASSERT(table.raw && (offset < INTREMAP_ENTRIES)); - ASSERT( (table != NULL) && (offset < INTREMAP_ENTRIES) ); + table.basic += offset; - return table + offset; + return table; } -static void free_intremap_entry(int seg, int bdf, int offset) +static void free_intremap_entry(unsigned int seg, unsigned int bdf, unsigned int offset) { - u32 *entry = get_intremap_entry(seg, bdf, offset); + union irte_ptr entry = get_intremap_entry(seg, bdf, offset); + + *entry.basic = (struct irte_basic){}; - memset(entry, 0, sizeof(u32)); __clear_bit(offset, get_ivrs_mappings(seg)[bdf].intremap_inuse); } -static void update_intremap_entry(u32* entry, u8 vector, u8 int_type, - u8 dest_mode, u8 dest) -{ - set_field_in_reg_u32(IOMMU_CONTROL_ENABLED, 0, - INT_REMAP_ENTRY_REMAPEN_MASK, - INT_REMAP_ENTRY_REMAPEN_SHIFT, entry); - set_field_in_reg_u32(IOMMU_CONTROL_DISABLED, *entry, - INT_REMAP_ENTRY_SUPIOPF_MASK, - INT_REMAP_ENTRY_SUPIOPF_SHIFT, entry); - set_field_in_reg_u32(int_type, *entry, - INT_REMAP_ENTRY_INTTYPE_MASK, - INT_REMAP_ENTRY_INTTYPE_SHIFT, entry); - set_field_in_reg_u32(IOMMU_CONTROL_DISABLED, *entry, - INT_REMAP_ENTRY_REQEOI_MASK, - INT_REMAP_ENTRY_REQEOI_SHIFT, entry); - set_field_in_reg_u32((u32)dest_mode, *entry, - INT_REMAP_ENTRY_DM_MASK, - INT_REMAP_ENTRY_DM_SHIFT, entry); - set_field_in_reg_u32((u32)dest, *entry, - INT_REMAP_ENTRY_DEST_MAST, - INT_REMAP_ENTRY_DEST_SHIFT, entry); - set_field_in_reg_u32((u32)vector, *entry, - INT_REMAP_ENTRY_VECTOR_MASK, - INT_REMAP_ENTRY_VECTOR_SHIFT, entry); +static void update_intremap_entry(union irte_ptr entry, unsigned int vector, + unsigned int int_type, + unsigned int dest_mode, unsigned int dest) +{ + struct irte_basic basic = { + .remap_en = 1, + .sup_io_pf = 0, + .int_type = int_type, + .rq_eoi = 0, + .dm = dest_mode, + .dest = dest, + .vector = vector, + }; + + *entry.basic = basic; } static inline int get_rte_index(const struct IO_APIC_route_entry *rte) @@ -163,7 +177,7 @@ static int update_intremap_entry_from_io u16 *index) { unsigned long flags; - u32* entry; + union irte_ptr entry; u8 delivery_mode, dest, vector, dest_mode; int req_id; spinlock_t *lock; @@ -201,12 +215,8 @@ static int update_intremap_entry_from_io * so need to recover vector and delivery mode from IRTE. */ ASSERT(get_rte_index(rte) == offset); - vector = get_field_from_reg_u32(*entry, - INT_REMAP_ENTRY_VECTOR_MASK, - INT_REMAP_ENTRY_VECTOR_SHIFT); - delivery_mode = get_field_from_reg_u32(*entry, - INT_REMAP_ENTRY_INTTYPE_MASK, - INT_REMAP_ENTRY_INTTYPE_SHIFT); + vector = entry.basic->vector; + delivery_mode = entry.basic->int_type; } update_intremap_entry(entry, vector, delivery_mode, dest_mode, dest); @@ -228,7 +238,7 @@ int __init amd_iommu_setup_ioapic_remapp { struct IO_APIC_route_entry rte; unsigned long flags; - u32* entry; + union irte_ptr entry; int apic, pin; u8 delivery_mode, dest, vector, dest_mode; u16 seg, bdf, req_id; @@ -407,16 +417,12 @@ unsigned int amd_iommu_read_ioapic_from_ u16 bdf = ioapic_sbdf[idx].bdf; u16 seg = ioapic_sbdf[idx].seg; u16 req_id = get_intremap_requestor_id(seg, bdf); - const u32 *entry = get_intremap_entry(seg, req_id, offset); + union irte_ptr entry = get_intremap_entry(seg, req_id, offset); ASSERT(offset == (val & (INTREMAP_ENTRIES - 1))); val &= ~(INTREMAP_ENTRIES - 1); - val |= get_field_from_reg_u32(*entry, - INT_REMAP_ENTRY_INTTYPE_MASK, - INT_REMAP_ENTRY_INTTYPE_SHIFT) << 8; - val |= get_field_from_reg_u32(*entry, - INT_REMAP_ENTRY_VECTOR_MASK, - INT_REMAP_ENTRY_VECTOR_SHIFT); + val |= MASK_INSR(entry.basic->int_type, IO_APIC_REDIR_DELIV_MODE_MASK); + val |= MASK_INSR(entry.basic->vector, IO_APIC_REDIR_VECTOR_MASK); } return val; @@ -427,7 +433,7 @@ static int update_intremap_entry_from_ms int *remap_index, const struct msi_msg *msg, u32 *data) { unsigned long flags; - u32* entry; + union irte_ptr entry; u16 req_id, alias_id; u8 delivery_mode, dest, vector, dest_mode; spinlock_t *lock; @@ -581,7 +587,7 @@ void amd_iommu_read_msi_from_ire( const struct pci_dev *pdev = msi_desc->dev; u16 bdf = pdev ? PCI_BDF2(pdev->bus, pdev->devfn) : hpet_sbdf.bdf; u16 seg = pdev ? pdev->seg : hpet_sbdf.seg; - const u32 *entry; + union irte_ptr entry; if ( IS_ERR_OR_NULL(_find_iommu_for_device(seg, bdf)) ) return; @@ -597,12 +603,8 @@ void amd_iommu_read_msi_from_ire( } msg->data &= ~(INTREMAP_ENTRIES - 1); - msg->data |= get_field_from_reg_u32(*entry, - INT_REMAP_ENTRY_INTTYPE_MASK, - INT_REMAP_ENTRY_INTTYPE_SHIFT) << 8; - msg->data |= get_field_from_reg_u32(*entry, - INT_REMAP_ENTRY_VECTOR_MASK, - INT_REMAP_ENTRY_VECTOR_SHIFT); + msg->data |= MASK_INSR(entry.basic->int_type, MSI_DATA_DELIVERY_MODE_MASK); + msg->data |= MASK_INSR(entry.basic->vector, MSI_DATA_VECTOR_MASK); } int __init amd_iommu_free_intremap_table( --- a/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h @@ -468,22 +468,6 @@ struct amd_iommu_pte { #define IOMMU_CONTROL_DISABLED 0 #define IOMMU_CONTROL_ENABLED 1 -/* interrupt remapping table */ -#define INT_REMAP_ENTRY_REMAPEN_MASK 0x00000001 -#define INT_REMAP_ENTRY_REMAPEN_SHIFT 0 -#define INT_REMAP_ENTRY_SUPIOPF_MASK 0x00000002 -#define INT_REMAP_ENTRY_SUPIOPF_SHIFT 1 -#define INT_REMAP_ENTRY_INTTYPE_MASK 0x0000001C -#define INT_REMAP_ENTRY_INTTYPE_SHIFT 2 -#define INT_REMAP_ENTRY_REQEOI_MASK 0x00000020 -#define INT_REMAP_ENTRY_REQEOI_SHIFT 5 -#define INT_REMAP_ENTRY_DM_MASK 0x00000040 -#define INT_REMAP_ENTRY_DM_SHIFT 6 -#define INT_REMAP_ENTRY_DEST_MAST 0x0000FF00 -#define INT_REMAP_ENTRY_DEST_SHIFT 8 -#define INT_REMAP_ENTRY_VECTOR_MASK 0x00FF0000 -#define INT_REMAP_ENTRY_VECTOR_SHIFT 16 - #define INV_IOMMU_ALL_PAGES_ADDRESS ((1ULL << 63) - 1) #define IOMMU_RING_BUFFER_PTR_MASK 0x0007FFF0
At the same time restrict its scope to just the single source file actually using it, and abstract accesses by introducing a union of pointers. (A union of the actual table entries is not used to make it impossible to [wrongly, once the 128-bit form gets added] perform pointer arithmetic / array accesses on derived types.) Also move away from updating the entries piecemeal: Construct a full new entry, and write it out. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- It would have been nice to use write_atomic() or ACCESS_ONCE() for the actual writes, but both cast the value to a scalar one, which doesn't suit us here (and I also didn't want to make the compound type a union with a raw member just for this).