[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 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 from translation_id read from firmwar MADT. This value is then programmed into 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 all the translator entries. There are a 3rd approach I suggested and ignored... The ITS entries for Dom0 is exactly the same as the host entries. So you only need to do a verbatim copy of the entry... Which of the two you find cleaner?This would also avoid to introduce a fake ID for DT as you currently do in patch #2.This can be avoided by storing translator_id only for acpi. +static int add_to_host_its_list(u64 addr, u64 size, + u32 translation_id, const void *node) +{ + struct host_its *its_data; + its_data = xzalloc(struct host_its); + + if ( !its_data ) + return -1; + + if ( node ) + its_data->dt_node = node; + else + its_data->translation_id = translation_id; + + its_data->addr = addr; + its_data->size = size; + printk("GICv3: Found ITS @0x%lx\n", addr); + + list_add_tail(&its_data->entry, &host_its_list); + + return 0; What do you think? I don't want to see the translation_id stored for no use at all but creating the DOM0 ACPI tables. Is that clearer? 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 |