diff mbox

[V2,7/25] tools/libacpi: Add new fields in acpi_config for DMAR table

Message ID 1502310866-10450-8-git-send-email-tianyu.lan@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

lan,Tianyu Aug. 9, 2017, 8:34 p.m. UTC
From: Chao Gao <chao.gao@intel.com>

The BIOS reports the remapping hardware units in a platform to system software
through the DMA Remapping Reporting (DMAR) ACPI table.
New fields are introduces for DMAR table. These new fields are set by
toolstack through parsing guest's config file. construct_dmar() is added to
build DMAR table according to the new fields.

Signed-off-by: Chao Gao <chao.gao@intel.com>
Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
 tools/libacpi/build.c   | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
 tools/libacpi/libacpi.h |  9 ++++++++
 2 files changed, 66 insertions(+)

Comments

Wei Liu Aug. 22, 2017, 1:12 p.m. UTC | #1
On Wed, Aug 09, 2017 at 04:34:08PM -0400, Lan Tianyu wrote:
> From: Chao Gao <chao.gao@intel.com>
> 
> The BIOS reports the remapping hardware units in a platform to system software
> through the DMA Remapping Reporting (DMAR) ACPI table.
> New fields are introduces for DMAR table. These new fields are set by
> toolstack through parsing guest's config file. construct_dmar() is added to
> build DMAR table according to the new fields.
> 
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> ---
>  tools/libacpi/build.c   | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
>  tools/libacpi/libacpi.h |  9 ++++++++
>  2 files changed, 66 insertions(+)
> 
> diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c
> index f9881c9..c7cc784 100644
> --- a/tools/libacpi/build.c
> +++ b/tools/libacpi/build.c
> @@ -28,6 +28,10 @@
>  
>  #define ACPI_MAX_SECONDARY_TABLES 16
>  
> +#define VTD_HOST_ADDRESS_WIDTH 39
> +#define I440_PSEUDO_BUS_PLATFORM 0xff
> +#define I440_PSEUDO_DEVFN_IOAPIC 0x0

I have some stupid questions. What are these I440 values? Where do they
come from?
Roger Pau Monné Aug. 22, 2017, 4:41 p.m. UTC | #2
On Wed, Aug 09, 2017 at 04:34:08PM -0400, Lan Tianyu wrote:
> From: Chao Gao <chao.gao@intel.com>
> 
> The BIOS reports the remapping hardware units in a platform to system software
> through the DMA Remapping Reporting (DMAR) ACPI table.
> New fields are introduces for DMAR table. These new fields are set by
                          ^ s/s/d/ to libacpi
> toolstack through parsing guest's config file. construct_dmar() is added to
> build DMAR table according to the new fields.
> 
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> ---
>  tools/libacpi/build.c   | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
>  tools/libacpi/libacpi.h |  9 ++++++++
>  2 files changed, 66 insertions(+)
> 
> diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c
> index f9881c9..c7cc784 100644
> --- a/tools/libacpi/build.c
> +++ b/tools/libacpi/build.c
> @@ -28,6 +28,10 @@
>  
>  #define ACPI_MAX_SECONDARY_TABLES 16

A comment about the meaning of the defines below might be helpful.

> +#define VTD_HOST_ADDRESS_WIDTH 39
> +#define I440_PSEUDO_BUS_PLATFORM 0xff
> +#define I440_PSEUDO_DEVFN_IOAPIC 0x0
> +
>  #define align16(sz)        (((sz) + 15) & ~15)
>  #define fixed_strcpy(d, s) strncpy((d), (s), sizeof(d))
>  
> @@ -303,6 +307,59 @@ static struct acpi_20_slit *construct_slit(struct acpi_ctxt *ctxt,
>      return slit;
>  }
>  
> +/*
> + * Only one DMA remapping hardware unit is exposed and all devices
> + * are under the remapping hardware unit. I/O APIC should be explicitly
> + * enumerated.
> + */
> +struct acpi_dmar *construct_dmar(struct acpi_ctxt *ctxt,
> +                                 const struct acpi_config *config)
> +{
> +    struct acpi_dmar *dmar;
> +    struct acpi_dmar_hardware_unit *drhd;
> +    struct dmar_device_scope *scope;
> +    unsigned int size;
> +    unsigned int ioapic_scope_size = sizeof(*scope) + sizeof(scope->path[0]);
> +
> +    size = sizeof(*dmar) + sizeof(*drhd) + ioapic_scope_size;
> +
> +    dmar = ctxt->mem_ops.alloc(ctxt, size, 16);
> +    if ( !dmar )
> +        return NULL;
> +
> +    memset(dmar, 0, size);
> +    dmar->header.signature = ACPI_2_0_DMAR_SIGNATURE;
> +    dmar->header.revision = ACPI_2_0_DMAR_REVISION;
> +    dmar->header.length = size;
> +    fixed_strcpy(dmar->header.oem_id, ACPI_OEM_ID);
> +    fixed_strcpy(dmar->header.oem_table_id, ACPI_OEM_TABLE_ID);
> +    dmar->header.oem_revision = ACPI_OEM_REVISION;
> +    dmar->header.creator_id   = ACPI_CREATOR_ID;
> +    dmar->header.creator_revision = ACPI_CREATOR_REVISION;
> +    dmar->host_address_width = VTD_HOST_ADDRESS_WIDTH - 1;
> +    if ( config->iommu_intremap_supported )
> +        dmar->flags = ACPI_DMAR_INTR_REMAP;

Since you initialize flags to 0 I would use |= here, in case this gets
moved later and this is not the first flag to be set.

> +    if ( !config->iommu_x2apic_supported )
> +        dmar->flags |= ACPI_DMAR_X2APIC_OPT_OUT;

I'm not sure of the reason behind not supporting x2APIC mode (I've
already commented in another patch).

> +    drhd = (struct acpi_dmar_hardware_unit *)((void*)dmar + sizeof(*dmar));
> +    drhd->type = ACPI_DMAR_TYPE_HARDWARE_UNIT;
> +    drhd->length = sizeof(*drhd) + ioapic_scope_size;
> +    drhd->flags = ACPI_DMAR_INCLUDE_PCI_ALL;
> +    drhd->pci_segment = 0;
> +    drhd->base_address = config->iommu_base_addr;
> +
> +    scope = &drhd->scope[0];
> +    scope->type = ACPI_DMAR_DEVICE_SCOPE_IOAPIC;
> +    scope->length = ioapic_scope_size;
> +    scope->enumeration_id = config->ioapic_id;
> +    scope->bus = I440_PSEUDO_BUS_PLATFORM;
> +    scope->path[0] = I440_PSEUDO_DEVFN_IOAPIC;

I'm not sure whether this constants should instead be fields in the
acpi_config struct passed down from libxl. libxc shouldn't really need
to know anything about which chipset a VM is using.

> +    set_checksum(dmar, offsetof(struct acpi_header, checksum), size);
> +    return dmar;
> +}
> +
>  static int construct_passthrough_tables(struct acpi_ctxt *ctxt,
>                                          unsigned long *table_ptrs,
>                                          int nr_tables,
> diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h
> index 2ed1ecf..74778a5 100644
> --- a/tools/libacpi/libacpi.h
> +++ b/tools/libacpi/libacpi.h
> @@ -20,6 +20,8 @@
>  #ifndef __LIBACPI_H__
>  #define __LIBACPI_H__
>  
> +#include <stdbool.h>
> +
>  #define ACPI_HAS_COM1              (1<<0)
>  #define ACPI_HAS_COM2              (1<<1)
>  #define ACPI_HAS_LPT1              (1<<2)
> @@ -96,8 +98,15 @@ struct acpi_config {
>      uint32_t ioapic_base_address;
>      uint16_t pci_isa_irq_mask;
>      uint8_t ioapic_id;
> +
> +    /* Emulated IOMMU features and location */
> +    bool iommu_intremap_supported;
> +    bool iommu_x2apic_supported;
> +    uint64_t iommu_base_addr;
>  };
>  
> +struct acpi_dmar *construct_dmar(struct acpi_ctxt *ctxt,
> +                                 const struct acpi_config *config);
>  int acpi_build_tables(struct acpi_ctxt *ctxt, struct acpi_config *config);
>  
>  #endif /* __LIBACPI_H__ */
> -- 
> 1.8.3.1
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
lan,Tianyu Aug. 23, 2017, 2:36 a.m. UTC | #3
On 2017年08月22日 21:12, Wei Liu wrote:
> On Wed, Aug 09, 2017 at 04:34:08PM -0400, Lan Tianyu wrote:
>> From: Chao Gao <chao.gao@intel.com>
>>
>> The BIOS reports the remapping hardware units in a platform to system software
>> through the DMA Remapping Reporting (DMAR) ACPI table.
>> New fields are introduces for DMAR table. These new fields are set by
>> toolstack through parsing guest's config file. construct_dmar() is added to
>> build DMAR table according to the new fields.
>>
>> Signed-off-by: Chao Gao <chao.gao@intel.com>
>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
>> ---
>>  tools/libacpi/build.c   | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  tools/libacpi/libacpi.h |  9 ++++++++
>>  2 files changed, 66 insertions(+)
>>
>> diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c
>> index f9881c9..c7cc784 100644
>> --- a/tools/libacpi/build.c
>> +++ b/tools/libacpi/build.c
>> @@ -28,6 +28,10 @@
>>  
>>  #define ACPI_MAX_SECONDARY_TABLES 16
>>  
>> +#define VTD_HOST_ADDRESS_WIDTH 39
>> +#define I440_PSEUDO_BUS_PLATFORM 0xff
>> +#define I440_PSEUDO_DEVFN_IOAPIC 0x0
> 
> I have some stupid questions. What are these I440 values? Where do they
> come from?
> 

Each IOAPIC device in the system reported via ACPI MADT must be
explicitly enumerated under one specific remapping hardware unit. We
assigned IOAPCI unit to bdf ff:00 and referenced Qemu vIOMMU implementation.
lan,Tianyu Aug. 23, 2017, 7:52 a.m. UTC | #4
On 2017年08月23日 00:41, Roger Pau Monné wrote:
>> > +    drhd = (struct acpi_dmar_hardware_unit *)((void*)dmar + sizeof(*dmar));
>> > +    drhd->type = ACPI_DMAR_TYPE_HARDWARE_UNIT;
>> > +    drhd->length = sizeof(*drhd) + ioapic_scope_size;
>> > +    drhd->flags = ACPI_DMAR_INCLUDE_PCI_ALL;
>> > +    drhd->pci_segment = 0;
>> > +    drhd->base_address = config->iommu_base_addr;
>> > +
>> > +    scope = &drhd->scope[0];
>> > +    scope->type = ACPI_DMAR_DEVICE_SCOPE_IOAPIC;
>> > +    scope->length = ioapic_scope_size;
>> > +    scope->enumeration_id = config->ioapic_id;
>> > +    scope->bus = I440_PSEUDO_BUS_PLATFORM;
>> > +    scope->path[0] = I440_PSEUDO_DEVFN_IOAPIC;
> I'm not sure whether this constants should instead be fields in the
> acpi_config struct passed down from libxl. libxc shouldn't really need
> to know anything about which chipset a VM is using.

How about rename I440_PSEUDO_XXX to VIOMMU_PSEUDO_XXX?
Roger Pau Monné Aug. 23, 2017, 8:04 a.m. UTC | #5
On Wed, Aug 23, 2017 at 03:52:01PM +0800, Lan Tianyu wrote:
> On 2017年08月23日 00:41, Roger Pau Monné wrote:
> >> > +    drhd = (struct acpi_dmar_hardware_unit *)((void*)dmar + sizeof(*dmar));
> >> > +    drhd->type = ACPI_DMAR_TYPE_HARDWARE_UNIT;
> >> > +    drhd->length = sizeof(*drhd) + ioapic_scope_size;
> >> > +    drhd->flags = ACPI_DMAR_INCLUDE_PCI_ALL;
> >> > +    drhd->pci_segment = 0;
> >> > +    drhd->base_address = config->iommu_base_addr;
> >> > +
> >> > +    scope = &drhd->scope[0];
> >> > +    scope->type = ACPI_DMAR_DEVICE_SCOPE_IOAPIC;
> >> > +    scope->length = ioapic_scope_size;
> >> > +    scope->enumeration_id = config->ioapic_id;
> >> > +    scope->bus = I440_PSEUDO_BUS_PLATFORM;
> >> > +    scope->path[0] = I440_PSEUDO_DEVFN_IOAPIC;
> > I'm not sure whether this constants should instead be fields in the
> > acpi_config struct passed down from libxl. libxc shouldn't really need
> > to know anything about which chipset a VM is using.
> 
> How about rename I440_PSEUDO_XXX to VIOMMU_PSEUDO_XXX?

I'm not really complaining about the naming, I'm just saying that I'm
not sure whether this constants should live in libxl. It would be
better IMHO if they where defined in some libxl x86 specific header,
and passed to libxc inside of the acpi_config struct.

At the end it is libxl which decides which chipset the VM is going to
use, not libxc.

Roger.
Wei Liu Aug. 23, 2017, 8:07 a.m. UTC | #6
On Wed, Aug 23, 2017 at 10:36:45AM +0800, Lan Tianyu wrote:
> On 2017年08月22日 21:12, Wei Liu wrote:
> > On Wed, Aug 09, 2017 at 04:34:08PM -0400, Lan Tianyu wrote:
> >> From: Chao Gao <chao.gao@intel.com>
> >>
> >> The BIOS reports the remapping hardware units in a platform to system software
> >> through the DMA Remapping Reporting (DMAR) ACPI table.
> >> New fields are introduces for DMAR table. These new fields are set by
> >> toolstack through parsing guest's config file. construct_dmar() is added to
> >> build DMAR table according to the new fields.
> >>
> >> Signed-off-by: Chao Gao <chao.gao@intel.com>
> >> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> >> ---
> >>  tools/libacpi/build.c   | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
> >>  tools/libacpi/libacpi.h |  9 ++++++++
> >>  2 files changed, 66 insertions(+)
> >>
> >> diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c
> >> index f9881c9..c7cc784 100644
> >> --- a/tools/libacpi/build.c
> >> +++ b/tools/libacpi/build.c
> >> @@ -28,6 +28,10 @@
> >>  
> >>  #define ACPI_MAX_SECONDARY_TABLES 16
> >>  
> >> +#define VTD_HOST_ADDRESS_WIDTH 39
> >> +#define I440_PSEUDO_BUS_PLATFORM 0xff
> >> +#define I440_PSEUDO_DEVFN_IOAPIC 0x0
> > 
> > I have some stupid questions. What are these I440 values? Where do they
> > come from?
> > 
> 
> Each IOAPIC device in the system reported via ACPI MADT must be
> explicitly enumerated under one specific remapping hardware unit. We
> assigned IOAPCI unit to bdf ff:00 and referenced Qemu vIOMMU implementation.

OK, do they need to be somewhere in a public header?

(See Roger's comment in the other sub-thread)
Roger Pau Monné Aug. 23, 2017, 2:11 p.m. UTC | #7
Small mistake in my message.

On Wed, Aug 23, 2017 at 09:04:06AM +0100, Roger Pau Monné wrote:
> On Wed, Aug 23, 2017 at 03:52:01PM +0800, Lan Tianyu wrote:
> > On 2017年08月23日 00:41, Roger Pau Monné wrote:
> > >> > +    drhd = (struct acpi_dmar_hardware_unit *)((void*)dmar + sizeof(*dmar));
> > >> > +    drhd->type = ACPI_DMAR_TYPE_HARDWARE_UNIT;
> > >> > +    drhd->length = sizeof(*drhd) + ioapic_scope_size;
> > >> > +    drhd->flags = ACPI_DMAR_INCLUDE_PCI_ALL;
> > >> > +    drhd->pci_segment = 0;
> > >> > +    drhd->base_address = config->iommu_base_addr;
> > >> > +
> > >> > +    scope = &drhd->scope[0];
> > >> > +    scope->type = ACPI_DMAR_DEVICE_SCOPE_IOAPIC;
> > >> > +    scope->length = ioapic_scope_size;
> > >> > +    scope->enumeration_id = config->ioapic_id;
> > >> > +    scope->bus = I440_PSEUDO_BUS_PLATFORM;
> > >> > +    scope->path[0] = I440_PSEUDO_DEVFN_IOAPIC;
> > > I'm not sure whether this constants should instead be fields in the
> > > acpi_config struct passed down from libxl. libxc shouldn't really need
> > > to know anything about which chipset a VM is using.
> > 
> > How about rename I440_PSEUDO_XXX to VIOMMU_PSEUDO_XXX?
> 
> I'm not really complaining about the naming, I'm just saying that I'm
> not sure whether this constants should live in libxl. It would be
                                                 ^ libxc
> better IMHO if they where defined in some libxl x86 specific header,
> and passed to libxc inside of the acpi_config struct.
> 
> At the end it is libxl which decides which chipset the VM is going to
> use, not libxc.
> 
> Roger.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
lan,Tianyu Aug. 24, 2017, 2:33 a.m. UTC | #8
On 2017年08月23日 16:04, Roger Pau Monné wrote:
> On Wed, Aug 23, 2017 at 03:52:01PM +0800, Lan Tianyu wrote:
>> On 2017年08月23日 00:41, Roger Pau Monné wrote:
>>>>> +    drhd = (struct acpi_dmar_hardware_unit *)((void*)dmar + sizeof(*dmar));
>>>>> +    drhd->type = ACPI_DMAR_TYPE_HARDWARE_UNIT;
>>>>> +    drhd->length = sizeof(*drhd) + ioapic_scope_size;
>>>>> +    drhd->flags = ACPI_DMAR_INCLUDE_PCI_ALL;
>>>>> +    drhd->pci_segment = 0;
>>>>> +    drhd->base_address = config->iommu_base_addr;
>>>>> +
>>>>> +    scope = &drhd->scope[0];
>>>>> +    scope->type = ACPI_DMAR_DEVICE_SCOPE_IOAPIC;
>>>>> +    scope->length = ioapic_scope_size;
>>>>> +    scope->enumeration_id = config->ioapic_id;
>>>>> +    scope->bus = I440_PSEUDO_BUS_PLATFORM;
>>>>> +    scope->path[0] = I440_PSEUDO_DEVFN_IOAPIC;
>>> I'm not sure whether this constants should instead be fields in the
>>> acpi_config struct passed down from libxl. libxc shouldn't really need
>>> to know anything about which chipset a VM is using.
>>
>> How about rename I440_PSEUDO_XXX to VIOMMU_PSEUDO_XXX?
> 
> I'm not really complaining about the naming, I'm just saying that I'm
> not sure whether this constants should live in libxl. It would be
> better IMHO if they where defined in some libxl x86 specific header,
> and passed to libxc inside of the acpi_config struct.
> 
> At the end it is libxl which decides which chipset the VM is going to
> use, not libxc.

We can do that but the bdf is reserved for IOAPIC and should be same for
different chipset. Do we still need to pass it via acpi_config?


> 
> Roger.
>
Jan Beulich Aug. 24, 2017, 6:54 a.m. UTC | #9
>>> On 24.08.17 at 04:33, <tianyu.lan@intel.com> wrote:
> On 2017年08月23日 16:04, Roger Pau Monné wrote:
>> On Wed, Aug 23, 2017 at 03:52:01PM +0800, Lan Tianyu wrote:
>>> On 2017年08月23日 00:41, Roger Pau Monné wrote:
>>>>>> +    drhd = (struct acpi_dmar_hardware_unit *)((void*)dmar + sizeof(*dmar));
>>>>>> +    drhd->type = ACPI_DMAR_TYPE_HARDWARE_UNIT;
>>>>>> +    drhd->length = sizeof(*drhd) + ioapic_scope_size;
>>>>>> +    drhd->flags = ACPI_DMAR_INCLUDE_PCI_ALL;
>>>>>> +    drhd->pci_segment = 0;
>>>>>> +    drhd->base_address = config->iommu_base_addr;
>>>>>> +
>>>>>> +    scope = &drhd->scope[0];
>>>>>> +    scope->type = ACPI_DMAR_DEVICE_SCOPE_IOAPIC;
>>>>>> +    scope->length = ioapic_scope_size;
>>>>>> +    scope->enumeration_id = config->ioapic_id;
>>>>>> +    scope->bus = I440_PSEUDO_BUS_PLATFORM;
>>>>>> +    scope->path[0] = I440_PSEUDO_DEVFN_IOAPIC;
>>>> I'm not sure whether this constants should instead be fields in the
>>>> acpi_config struct passed down from libxl. libxc shouldn't really need
>>>> to know anything about which chipset a VM is using.
>>>
>>> How about rename I440_PSEUDO_XXX to VIOMMU_PSEUDO_XXX?
>> 
>> I'm not really complaining about the naming, I'm just saying that I'm
>> not sure whether this constants should live in libxl. It would be
>> better IMHO if they where defined in some libxl x86 specific header,
>> and passed to libxc inside of the acpi_config struct.
>> 
>> At the end it is libxl which decides which chipset the VM is going to
>> use, not libxc.
> 
> We can do that but the bdf is reserved for IOAPIC and should be same for
> different chipset. Do we still need to pass it via acpi_config?

Well, which value is the right (reserved) one surely can - at least
in theory - depend on the chipset. Which means that it should
come from the same place which determines the chipset to be
emulated for the guest.

Jan
lan,Tianyu Aug. 24, 2017, 8:36 a.m. UTC | #10
On 2017年08月24日 14:54, Jan Beulich wrote:
>>>> On 24.08.17 at 04:33, <tianyu.lan@intel.com> wrote:
>> On 2017年08月23日 16:04, Roger Pau Monné wrote:
>>> On Wed, Aug 23, 2017 at 03:52:01PM +0800, Lan Tianyu wrote:
>>>> On 2017年08月23日 00:41, Roger Pau Monné wrote:
>>>>>>> +    drhd = (struct acpi_dmar_hardware_unit *)((void*)dmar + sizeof(*dmar));
>>>>>>> +    drhd->type = ACPI_DMAR_TYPE_HARDWARE_UNIT;
>>>>>>> +    drhd->length = sizeof(*drhd) + ioapic_scope_size;
>>>>>>> +    drhd->flags = ACPI_DMAR_INCLUDE_PCI_ALL;
>>>>>>> +    drhd->pci_segment = 0;
>>>>>>> +    drhd->base_address = config->iommu_base_addr;
>>>>>>> +
>>>>>>> +    scope = &drhd->scope[0];
>>>>>>> +    scope->type = ACPI_DMAR_DEVICE_SCOPE_IOAPIC;
>>>>>>> +    scope->length = ioapic_scope_size;
>>>>>>> +    scope->enumeration_id = config->ioapic_id;
>>>>>>> +    scope->bus = I440_PSEUDO_BUS_PLATFORM;
>>>>>>> +    scope->path[0] = I440_PSEUDO_DEVFN_IOAPIC;
>>>>> I'm not sure whether this constants should instead be fields in the
>>>>> acpi_config struct passed down from libxl. libxc shouldn't really need
>>>>> to know anything about which chipset a VM is using.
>>>>
>>>> How about rename I440_PSEUDO_XXX to VIOMMU_PSEUDO_XXX?
>>>
>>> I'm not really complaining about the naming, I'm just saying that I'm
>>> not sure whether this constants should live in libxl. It would be
>>> better IMHO if they where defined in some libxl x86 specific header,
>>> and passed to libxc inside of the acpi_config struct.
>>>
>>> At the end it is libxl which decides which chipset the VM is going to
>>> use, not libxc.
>>
>> We can do that but the bdf is reserved for IOAPIC and should be same for
>> different chipset. Do we still need to pass it via acpi_config?
> 
> Well, which value is the right (reserved) one surely can - at least
> in theory - depend on the chipset. Which means that it should
> come from the same place which determines the chipset to be
> emulated for the guest.
> 

OK. Will update.
diff mbox

Patch

diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c
index f9881c9..c7cc784 100644
--- a/tools/libacpi/build.c
+++ b/tools/libacpi/build.c
@@ -28,6 +28,10 @@ 
 
 #define ACPI_MAX_SECONDARY_TABLES 16
 
+#define VTD_HOST_ADDRESS_WIDTH 39
+#define I440_PSEUDO_BUS_PLATFORM 0xff
+#define I440_PSEUDO_DEVFN_IOAPIC 0x0
+
 #define align16(sz)        (((sz) + 15) & ~15)
 #define fixed_strcpy(d, s) strncpy((d), (s), sizeof(d))
 
@@ -303,6 +307,59 @@  static struct acpi_20_slit *construct_slit(struct acpi_ctxt *ctxt,
     return slit;
 }
 
+/*
+ * Only one DMA remapping hardware unit is exposed and all devices
+ * are under the remapping hardware unit. I/O APIC should be explicitly
+ * enumerated.
+ */
+struct acpi_dmar *construct_dmar(struct acpi_ctxt *ctxt,
+                                 const struct acpi_config *config)
+{
+    struct acpi_dmar *dmar;
+    struct acpi_dmar_hardware_unit *drhd;
+    struct dmar_device_scope *scope;
+    unsigned int size;
+    unsigned int ioapic_scope_size = sizeof(*scope) + sizeof(scope->path[0]);
+
+    size = sizeof(*dmar) + sizeof(*drhd) + ioapic_scope_size;
+
+    dmar = ctxt->mem_ops.alloc(ctxt, size, 16);
+    if ( !dmar )
+        return NULL;
+
+    memset(dmar, 0, size);
+    dmar->header.signature = ACPI_2_0_DMAR_SIGNATURE;
+    dmar->header.revision = ACPI_2_0_DMAR_REVISION;
+    dmar->header.length = size;
+    fixed_strcpy(dmar->header.oem_id, ACPI_OEM_ID);
+    fixed_strcpy(dmar->header.oem_table_id, ACPI_OEM_TABLE_ID);
+    dmar->header.oem_revision = ACPI_OEM_REVISION;
+    dmar->header.creator_id   = ACPI_CREATOR_ID;
+    dmar->header.creator_revision = ACPI_CREATOR_REVISION;
+    dmar->host_address_width = VTD_HOST_ADDRESS_WIDTH - 1;
+    if ( config->iommu_intremap_supported )
+        dmar->flags = ACPI_DMAR_INTR_REMAP;
+    if ( !config->iommu_x2apic_supported )
+        dmar->flags |= ACPI_DMAR_X2APIC_OPT_OUT;
+
+    drhd = (struct acpi_dmar_hardware_unit *)((void*)dmar + sizeof(*dmar));
+    drhd->type = ACPI_DMAR_TYPE_HARDWARE_UNIT;
+    drhd->length = sizeof(*drhd) + ioapic_scope_size;
+    drhd->flags = ACPI_DMAR_INCLUDE_PCI_ALL;
+    drhd->pci_segment = 0;
+    drhd->base_address = config->iommu_base_addr;
+
+    scope = &drhd->scope[0];
+    scope->type = ACPI_DMAR_DEVICE_SCOPE_IOAPIC;
+    scope->length = ioapic_scope_size;
+    scope->enumeration_id = config->ioapic_id;
+    scope->bus = I440_PSEUDO_BUS_PLATFORM;
+    scope->path[0] = I440_PSEUDO_DEVFN_IOAPIC;
+
+    set_checksum(dmar, offsetof(struct acpi_header, checksum), size);
+    return dmar;
+}
+
 static int construct_passthrough_tables(struct acpi_ctxt *ctxt,
                                         unsigned long *table_ptrs,
                                         int nr_tables,
diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h
index 2ed1ecf..74778a5 100644
--- a/tools/libacpi/libacpi.h
+++ b/tools/libacpi/libacpi.h
@@ -20,6 +20,8 @@ 
 #ifndef __LIBACPI_H__
 #define __LIBACPI_H__
 
+#include <stdbool.h>
+
 #define ACPI_HAS_COM1              (1<<0)
 #define ACPI_HAS_COM2              (1<<1)
 #define ACPI_HAS_LPT1              (1<<2)
@@ -96,8 +98,15 @@  struct acpi_config {
     uint32_t ioapic_base_address;
     uint16_t pci_isa_irq_mask;
     uint8_t ioapic_id;
+
+    /* Emulated IOMMU features and location */
+    bool iommu_intremap_supported;
+    bool iommu_x2apic_supported;
+    uint64_t iommu_base_addr;
 };
 
+struct acpi_dmar *construct_dmar(struct acpi_ctxt *ctxt,
+                                 const struct acpi_config *config);
 int acpi_build_tables(struct acpi_ctxt *ctxt, struct acpi_config *config);
 
 #endif /* __LIBACPI_H__ */