Re: [PATCH V3 7/29] tools/libxl: build DMAR table for a guest with one virtual VTD

[Chao Gao <chao.gao@intel.com>]

On Thu, Oct 19, 2017 at 11:00:27AM +0100, Roger Pau Monné wrote:
>On Thu, Sep 21, 2017 at 11:01:48PM -0400, Lan Tianyu wrote:
>> From: Chao Gao <chao.gao@intel.com>
>> 
>> A new logic is added to build ACPI DMAR table in tool stack for a guest
>> with one virtual VTD and pass through it to guest via existing mechanism. If
>> there already are ACPI tables needed to pass through, we joint the tables.
>> 
>> Signed-off-by: Chao Gao <chao.gao@intel.com>
>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
>> 
>> ---
>> +/*
>> + * For hvm, we don't need build acpi in libxl. Instead, it's built in hvmloader.
>> + * But if one hvm has virtual VTD(s), we build DMAR table for it and joint this
>> + * table with existing content in acpi_modules in order to employ HVM
>> + * firmware pass-through mechanism to pass-through DMAR table.
>> + */
>> +static int libxl__dom_load_acpi_hvm(libxl__gc *gc,
>> +                                    const libxl_domain_build_info *b_info,
>> +                                    struct xc_dom_image *dom)
>> +{
>
>AFAICT there's some code duplication between libxl__dom_load_acpi_hvm
>and libxl__dom_load_acpi_pvh, isn't there a chance you could put this
>in a common function?

Will give it a shot.

>
>> +    struct acpi_config config = { 0 };
>> +    struct acpi_ctxt ctxt;
>> +    void *table;
>> +    uint32_t len;
>> +
>> +    if ((b_info->type != LIBXL_DOMAIN_TYPE_HVM) ||
>> +        (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_NONE) ||
>> +        (b_info->num_viommus != 1) ||
>> +        (b_info->viommu[0].type != LIBXL_VIOMMU_TYPE_INTEL_VTD))
>> +        return 0;
>> +
>> +    ctxt.mem_ops.alloc = acpi_memalign;
>> +    ctxt.mem_ops.v2p = virt_to_phys;
>> +    ctxt.mem_ops.free = acpi_mem_free;
>> +
>> +    if (libxl_defbool_val(b_info->viommu[0].intremap))
>> +        config.iommu_intremap_supported = true;
>> +    /* x2apic is always enabled since in no case we must disable it */
>> +    config.iommu_x2apic_supported = true;
>> +    config.iommu_base_addr = b_info->viommu[0].base_addr;
>
>I don't see libxl__dom_load_acpi_pvh setting any of the vIOMMU fields.

I didn't try to enable vIOMMU for PVH. I will attemp to add vIOMMU
support for PVH and put those patches at the end of this series. 

>
>> +int libxl__dom_load_acpi(libxl__gc *gc,
>> +                         const libxl_domain_build_info *b_info,
>> +                         struct xc_dom_image *dom)
>> +{
>> +
>> +    if (b_info->type != LIBXL_DOMAIN_TYPE_HVM)
>> +        return 0;
>
>Keep in mind a new PVH domain type has been introduced recently in
>libxl, you will have to change this to b_info->type == LIBXL_DOMAIN_TYPE_PV.

Thanks for your kind reminder.

Chao


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel