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

[Peter Xu <peterx@redhat.com>]

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" <mst@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
> Cc: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  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?

Thanks,

> +    }
> +
>      build_header(linker, table_data, (void *)(table_data->data + iommu_start),
>                   "IVRS", table_data->len - iommu_start, 1, NULL, NULL);
>  }
> -- 
> 2.7.4
> 
> 

-- 
Peter Xu