Re: [Qemu-devel] [PATCH 4/6] i386: acpi: add IVHD device entry for IOAPIC

[Brijesh Singh]

On 09/11/2018 11:35 PM, Peter Xu wrote:
> On Tue, Sep 11, 2018 at 11:49:47AM -0500, Brijesh Singh wrote:
> > When interrupt remapping is enabled, add a special IVHD device
> > (type IOAPIC) -- which is typically PCI device 14:0.0. Linux IOMMU driver
> > checks for this special device.
> >
> > Cc: "Michael S. Tsirkin" <address@hidden>
> > Cc: Paolo Bonzini <address@hidden>
> > Cc: Richard Henderson <address@hidden>
> > Cc: Eduardo Habkost <address@hidden>
> > Cc: Marcel Apfelbaum <address@hidden>
> > Cc: Tom Lendacky <address@hidden>
> > Cc: Suravee Suthikulpanit <address@hidden>
> > Signed-off-by: Brijesh Singh <address@hidden>
> > ---
> >   hw/i386/acpi-build.c | 20 +++++++++++++++++++-
> >   1 file changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index e1ee8ae..5c2c638 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -2519,6 +2519,7 @@ build_dmar_q35(GArray *table_data, BIOSLinker *linker)
> >   static void
> >   build_amd_iommu(GArray *table_data, BIOSLinker *linker)
> >   {
> > +    int ivhd_table_len = 28;
> >       int iommu_start = table_data->len;
> >       AMDVIState *s = AMD_IOMMU_DEVICE(x86_iommu_get_default());
> >   
> > @@ -2540,8 +2541,16 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker)
> >                                (1UL << 6) | /* PrefSup      */
> >                                (1UL << 7),  /* PPRSup       */
> >                                1);
> > +
> > +    /*
> > +     * When interrupt remapping is enabled, we add a special IVHD device
> > +     * for type IO-APIC.
> > +     */
> > +    if (s->intr_enabled) {
> > +        ivhd_table_len += 8;
> > +    }
> >       /* IVHD length */
> > -    build_append_int_noprefix(table_data, 28, 2);
> > +    build_append_int_noprefix(table_data, ivhd_table_len, 2);
> >       /* DeviceID */
> >       build_append_int_noprefix(table_data, s->devid, 2);
> >       /* Capability offset */
> > @@ -2565,6 +2574,15 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker)
> >        */
> >       build_append_int_noprefix(table_data, 0x0000001, 4);
> >   
> > +    /*
> > +     * When interrupt remapping is enabled, Linux IOMMU driver also checks
> > +     * for special IVHD device (type IO-APIC), which is typically presented
> > +     * as PCI device 14:00.0.
> > +     */
> > +    if (s->intr_enabled) {
> > +        build_append_int_noprefix(table_data, 0x0100a00000000048, 8);
>
> Some comments on the bit definition would be nicer, or "please refer
> to Table 95 of AMD-Vi spec".
>
> Could I ask how come the 14:00.0?  Is that in the spec somewhere?
>
> And since you explicitly mentioned Linux, then... would it work for
> Windows too?

The PCI 14:00.0 is SouthBridge IOAPIC device. On bare metal the timer
subsystem is connected to the SB IOAPIC. The IVRS table must contains
the entry of SB IOAPIC otherwise Linux will not enable the IR mapping
while parsing the IVRS.

On bare meta system, IVRS will always have entry for SB IOAPIC. As per
Windows is concerned, I am not sure if Windows support interrupt remap.
If it does, adding the SB IOAPIC devid should not cause any problem
to it because its always available on bare metal system.

Here is linux commit

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c2ff5cf5294bcbd7fa50f7d860e90a66db7e5059