Re: [PATCH RFC 3/6] x86/ioapic: RTE modifications must use ioapic_write_entry

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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.