[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 0/4] ARM: ACPI: ITS: Add ITS Support for ACPI hardware domain
On 08/10/2017 02:00 PM, Manish Jaggi wrote: HI Julien, On 8/10/2017 5:43 PM, Julien Grall wrote:On 10/08/17 13:00, Manish Jaggi wrote:Hi Julien, On 8/10/2017 4:58 PM, Julien Grall wrote:On 10/08/17 12:21, Manish Jaggi wrote:Hi Julien, On 6/21/2017 6:53 PM, Julien Grall wrote:Hi Manish, On 21/06/17 02:01, Manish Jaggi wrote:This patch series adds the support of ITS for ACPI hardware domain. It is tested on staging branch with has ITS v12 patchset by Andre. I have tried to incorporate the review comments on the RFC v1/v2 patch. The single patch in RFC is now split into 4 patches.I will comment here rather than on each patches.Patch1: ARM: ITS: Add translation_id to host_its Adds translation_id in host_its data structure, which is populated fromtranslation_id read from firmwar MADT. This value is then programmedinto local MADT created for hardware domain in patch 4.I don't see any reason to store value that will only be used for generating the MADT which BTW is just a copy for the ITS. Instead we should copy over the MADT entries.There are two approaches, If I use the standard API acpi_table_parse_madt which would iterate over ACPI_MADT_TYPE_GENERIC_TRANSLATOR entries, I have to maintain the addr and translation_id in some data structure, to be filled later in the hwdomain copy of madt generic translator.If I don't use the standard API I have to add code to manually parse allthe translator entries.There are a 3rd approach I suggested and ignored... The ITS entries for Dom0 is exactly the same as the host entries.Yes, and if not passed properly dom0 wont get device interrupts...So you only need to do a verbatim copy of the entry...Can you please check patch 4/2, the translation_id and address are passed verbatim, the other values are reserved in acpi_madt_generic_translator.For ACPI, we took the approach to only rewrite what's necessary and give the rest to Dom0 as it is. If newer version of ACPI re-used those fields, then they will be copied over to Dom0. I don't consider it as an issue because the problem would be the same if those fields have an important meaning for the platform.Few thoughts... If we follow this approach, few points needs to be considered- If ACPI may use the reserved information later it could be equally important for dom0 and Xen,so it might be useful to keep reserved in xen as well. I already covered that in my previous e-mail. - For platforms which use dt, translation_id is not required to be stored in struct host_its, similarly for platforms which use acpidt_node pointer might be of no use.So we can have struct host_its having a union with dt_device_node * for dt and acpi_madt_generic_translator * for acpi.IMHO this could be an approach we can take. struct host_its { struct list_head entry; - const struct dt_device_node *dt_node; + union { + const struct dt_device_node *dt_node; + const struct acpi_madt_generic_translator *acpi_its_entry; + }; paddr_t addr; What don't you get in my previous e-mail? A no is a no, full stop.Just do what we do in *_make_hwdom_madt. That will work here with no need of a union or anything else. Even the DT code can be reworked to avoid storing the node. We could even have the common code go through the MADT entry one by one and let the specific driver updating what's necessary avoid ACPI_MEMCPY duplication in each bit. But this is a longer term thoughts than for. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |