Re: [PATCH V2 9/25] tools/libxl: build DMAR table for a guest with one virtual VTD

[Lan Tianyu <tianyu.lan@intel.com>]

On 2017年08月25日 11:19, Lan Tianyu wrote:
> On 2017年08月24日 19:08, Wei Liu wrote:
>>>>> If add dmar table for hvmlite, we should combine dmar table with other
>>>>>>>> ACPI table and populate into acpi_modules[2]. This is how hvmlite add
>>>>>>>> other ACPI tables in libxl__dom_load_acpi().
>>>>>>>>
>>>>>>
>>>>>> Sure, that sounds plausible.
>>>>>>
>>>>>> What I would like to see is to have one entry point to manipulate APCI
>>>>>> tables.
>>>>>>
>>>>>> Given the patch volume we're seeing now, we expect contributors to drive
>>>>>> the discussion forward. If you're not sure, feel free to ask more questions.
>>>>
>>>> I am not sure whether I understood correctly.
>>>>
>>>> PVHv2 builds all ACPI table in tool stack and uses acpi_module[0, 1, 2]
>>>> to pass related table content.
>>>>
>>>> HVM builds ACPI tables in hvmloader and just use acpi_module[0] to pass
>>>> additional ACPI firmware or table.
>>>>
>>>> These two modes have different way to use acpi_modules[]. So I think we
>>>> can't combine them, right?
>>>>
>> There might be some misunderstanding.  We probably don't want to
>> manipulate the content of the tables in libxl.
>>
>>>> For build dmar table, we have introduced construct_dmar() in under
>>>> libacpi to build dmar table and PVHv2 also can use it in
>>>> libxl__dom_load_acpi().
>>>>
>> My major complain is now there are two functions and in two different
>> locations, in two different phases of domain construction that would
>> manipulate ACPI tables. I would like to have only one.
>>
>> The function you're currently modifying libxl__domain_firmware is not
>> the right place. It's primary function is to load files from disks.
>>
>> You should be able to call the function you introduced in
>> libxl__dom_load_acpi, provided appropriate checks are added.
> 
> But libxl__dom_load_acpi() isn't called on hvm guest code path. It just
> works for PVHv2/HVMlite and have some conflict with hvm guest
> configuration(i.e, acpi_module).
> 
> 
> int libxl__arch_domain_finalise_hw_description(libxl__gc *gc,
>                                                libxl_domain_build_info
> *info,
>                                                struct xc_dom_image *dom)
> {
>     int rc = 0;
> 
>     if ((info->type == LIBXL_DOMAIN_TYPE_HVM) &&
>         (info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_NONE)) {
>         rc = libxl__dom_load_acpi(gc, info, dom);
>         if (rc != 0)
>             LOGE(ERROR, "libxl_dom_load_acpi failed");
>     }
> 
>     return rc;
> }

We may remove the check and move introduced code in
libxl__dom_load_acpi(). Run new code just for hvm guest. Does this make
sense?

-- 
Best regards
Tianyu Lan

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