On Fri, Jun 03, 2022 at 03:34:33PM +0200, Jan Beulich wrote:
> On 21.04.2022 15:21, Roger Pau Monne wrote:
> > Do not allow to write to RTE registers using io_apic_write and instead
> > require changes to RTE to be performed using ioapic_write_entry.
>
> Hmm, this doubles the number of MMIO access in affected code fragments.
But it does allow to simplify the IOMMU side quite a lot by no longer
having to update the IRTE using two different calls. I'm quite sure
it saves quite some accesses to the IOMMU RTE in the following
patches.
> > --- a/xen/arch/x86/include/asm/io_apic.h
> > +++ b/xen/arch/x86/include/asm/io_apic.h
> > @@ -161,22 +161,11 @@ static inline void __io_apic_write(unsigned int apic,
> > unsigned int reg, unsigned
> >
> > static inline void io_apic_write(unsigned int apic, unsigned int reg,
> > unsigned int value)
> > {
> > - if ( ioapic_reg_remapped(reg) )
> > - return iommu_update_ire_from_apic(apic, reg, value);
> > + /* RTE writes must use ioapic_write_entry. */
> > + BUG_ON(reg >= 0x10);
> > __io_apic_write(apic, reg, value);
> > }
> >
> > -/*
> > - * Re-write a value: to be used for read-modify-write
> > - * cycles where the read already set up the index register.
> > - */
> > -static inline void io_apic_modify(unsigned int apic, unsigned int reg,
> > unsigned int value)
> > -{
> > - if ( ioapic_reg_remapped(reg) )
> > - return iommu_update_ire_from_apic(apic, reg, value);
> > - *(IO_APIC_BASE(apic) + 4) = value;
> > -}
>
> While the last caller goes away, I don't think this strictly needs to
> be dropped (but could just gain a BUG_ON() like you do a few lines up)?
Hm, could do, but it won't be suitable to be used to modify RTEs
anymore, and given that was it's only usage I didn't see much value
for leaving it around.
Thanks, Roger.
