[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v8 04/25] acpi: add DMAR scope definition for ro
From: |
Peter Xu |
Subject: |
Re: [Qemu-devel] [PATCH v8 04/25] acpi: add DMAR scope definition for root IOAPIC |
Date: |
Thu, 2 Jun 2016 12:16:20 +0800 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Wed, Jun 01, 2016 at 02:56:38PM +0200, Igor Mammedov wrote:
[...]
> > @@ -2561,6 +2563,9 @@ build_dmar_q35(MachineState *ms, GArray *table_data,
> > GArray *linker)
> > AcpiTableDmar *dmar;
> > AcpiDmarHardwareUnit *drhd;
> > uint8_t dmar_flags = 0;
> > + AcpiDmarDeviceScope *scope = NULL;
> > + /* Root complex IOAPIC use one path[0] only */
> > + uint16_t scope_size = sizeof(*scope) + sizeof(uint16_t);
> ioapic_scope_size
> sizeof(scope->path[0]) /* space for IOxAPIC path */
Agree on both.
>
> > if (ms->iommu_intr) {
> > /* enable INTR for the IOMMU device */
> > @@ -2574,11 +2579,19 @@ build_dmar_q35(MachineState *ms, GArray
> > *table_data, GArray *linker)
> > /* DMAR Remapping Hardware Unit Definition structure */
> > drhd = acpi_data_push(table_data, sizeof(*drhd));
> sizeof(*drhd) + ioapic_scope_size
>
> > drhd->type = cpu_to_le16(ACPI_DMAR_TYPE_HARDWARE_UNIT);
> > - drhd->length = cpu_to_le16(sizeof(*drhd)); /* No device scope now */
> > + drhd->length = cpu_to_le16(sizeof(*drhd) + scope_size);
> > drhd->flags = ACPI_DMAR_INCLUDE_PCI_ALL;
> > drhd->pci_segment = cpu_to_le16(0);
> > drhd->address = cpu_to_le64(Q35_HOST_BRIDGE_IOMMU_ADDR);
> >
> > + /* Scope definition for the root-complex IOAPIC */
> > + scope = acpi_data_push(table_data, scope_size);
> no need for this as previous push took care about all space we need for scope
> ioapic_scope = &drhd->scope[0];
Both work for me. I can do a switch.
>
> > + scope->entry_type = cpu_to_le16(ACPI_DMAR_DEV_SCOPE_TYPE_IOAPIC);
> entry_type is 1 byte long, doing cpu_to_le16 on bigendian host
> will always result in 0 being written there
>
> > + scope->length = scope_size;
> > + scope->enumeration_id = cpu_to_le16(ACPI_BUILD_IOAPIC_ID);
> > + scope->bus = cpu_to_le16(Q35_PSEUDO_BUS_PLATFORM);
> ditto for above 2 fields
Ah, didn't notice these lines after it worked. For the three places, I
should remove cpu_to_le16().
[...]
> > +/*
> > + * Arbitary but unique BNF number for IOAPIC device. This is only
> > + * used when interrupt remapping is enabled.
> you encode it in DMAR unconditionally so not "only"
Okay. Removing the 2nd sentence.
Thanks for the review comments, Igor!
-- peterx