Re: [PATCH 1/3] xen/vioapic: add support for the extended destination ID field

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

On Mon, Jan 24, 2022 at 02:20:47PM +0100, Jan Beulich wrote:
> On 20.01.2022 16:23, Roger Pau Monne wrote:
> > Such field uses bits 55:48, but for the purposes the register will be
> > used use bits 55:49 instead. Bit 48 is used to signal an RTE entry is
> > in remappable format which is not supported by the vIO-APIC.
> 
> Neither here nor in the cover letter you point at a formal specification
> of this mode of operation.

I'm not aware of any formal specification of this mode, apart from the
work done to introduce support in Linux and QEMU:

https://lore.kernel.org/all/20201009104616.1314746-1-dwmw2@xxxxxxxxxxxxx/
https://git.qemu.org/?p=qemu.git;a=commitdiff;h=c1bb5418e

Adding David in case there's some kind of specification somewhere I'm
not aware of.

> What I'm aware of are vague indications of
> this mode's existence in some of Intel's chipset data sheets. Yet that
> leaves open, for example, whether indeed bit 48 cannot be used here.

Bit 48 cannot be used because it's already used to signal an RTE is in
remappable format. We still want to differentiate an RTE entry in
remappable format, as it should be possible to expose both the
extended ID support and an emulated IOMMU.

> > --- a/xen/arch/x86/hvm/vioapic.c
> > +++ b/xen/arch/x86/hvm/vioapic.c
> > @@ -412,7 +412,8 @@ static void ioapic_inj_irq(
> >  
> >  static void vioapic_deliver(struct hvm_vioapic *vioapic, unsigned int pin)
> >  {
> > -    uint16_t dest = vioapic->redirtbl[pin].fields.dest_id;
> > +    uint16_t dest = vioapic->redirtbl[pin].fields.dest_id |
> > +                    (vioapic->redirtbl[pin].fields.ext_dest_id << 8);
> 
> What if an existing guest has been writing non-zero in these bits? Can
> you really use them here without any further indication by the guest?

Those bits where reserved previously, so no OS should have used them.
There are hypervisors already in the field (QEMU/KVM and HyperV) using
this mode.

We could add a per-domain option to disable extended ID mode if we are
really worried about OSes having used those bits for some reason.

Thanks, Roger.