Hi Prem, On 23/09/2016 16:07, Prem Mallappa wrote: > On Fri, Sep 23, 2016 at 6:40 PM, Auger Eric <eric.au...@redhat.com > <mailto:eric.au...@redhat.com>> wrote: > > Hi Prem, > > On 12/09/2016 22:42, Prem Mallappa wrote: > > On Fri, Sep 9, 2016 at 8:54 PM, Auger Eric <eric.au...@redhat.com > <mailto:eric.au...@redhat.com>> wrote: > > > >> Hi Prem, > >> > >> On 22/08/2016 18:17, Prem Mallappa wrote: > >>> Added ACPI IORT tables, was needed for internal project purpose, but > >>> posting here for anyone looking for testing ACPI on ARM platforms. > >>> (P.S: Linux side IORT patches are WIP) > >> I am also interested in IORT ITS group and currently prototyping > >> something, hence my few comments/questions. > >>> > >>> Signed-off-by: Prem Mallappa <prem.malla...@broadcom.com > <mailto:prem.malla...@broadcom.com>> > >>> --- > >>> hw/arm/virt-acpi-build.c | 43 +++++++++++++++++++++++ > >>> include/hw/acpi/acpi-defs.h | 84 ++++++++++++++++++++++++++++++ > >> +++++++++++++++ > >>> 2 files changed, 127 insertions(+) > >>> > >>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > >>> index 1fa0581..d5fb69e 100644 > >>> --- a/hw/arm/virt-acpi-build.c > >>> +++ b/hw/arm/virt-acpi-build.c > >>> @@ -382,6 +382,45 @@ build_rsdp(GArray *rsdp_table, BIOSLinker > *linker, > >> unsigned rsdt_tbl_offset) > >>> return rsdp_table; > >>> } > >>> > >>> +/* > >>> + * TODO: Simple IORT for now, will add ID mappings as we go > >>> + * basic idea is to instantiate SMMU from ACPI > >>> + */ > >>> +static void > >>> +build_iort(GArray *table_data, BIOSLinker *linker, VirtGuestInfo > >> *guest_info) > >>> +{ > >>> + int iort_start = table_data->len; > >>> + AcpiIortTable *iort; > >>> + AcpiIortNode *iort_node; > >>> + AcpiIortSmmu3 *smmu; > >>> + AcpiIortRC *rc; > >>> + const MemMapEntry *memmap = guest_info->memmap; > >>> + > >>> + iort = acpi_data_push(table_data, sizeof(*iort)); > >>> + > >>> + iort->length = sizeof(*iort); > >> Isn't is supposed to be the length of the whole IORT (including > the node > >> cumulated sizes?) > >>> + iort->node_offset = table_data->len - iort_start; > >>> + iort->num_nodes++; > >>> + > >>> + smmu = acpi_data_push(table_data, sizeof(*smmu)); > >>> + iort_node = &smmu->iort_node; > >>> + iort_node->type = 0x04; /* SMMUv3 */ > >> To match existing code (include/hw/acpi/acpi-defs.h), maybe enum > values > >> can be created (ACPI_IORT_NODE_SMMU_V3). This also matches kernel > enum. > >> > >> I have made these changes, will send out ASAP. > > > > > >> More generally Shannon advised to use the same field names as the > ones > >> used in the kernel header: acpi_iort_node_type in > include/acpi/actbl2.h > >> > > > > Will change this accordingly > > > > > >>> + iort_node->length = sizeof(*smmu); > >>> + smmu->base_addr = cpu_to_le64(memmap[VIRT_SMMU].base); > >>> + > >>> + iort->num_nodes++; > >>> + > >>> + rc = acpi_data_push(table_data, sizeof(*rc)); > >>> + iort_node = &rc->iort_node; > >>> + iort_node->type = 0x02; /* RC */ > >>> + iort_node->length = sizeof(*rc); > >> I think the mem_access_prop field should be set to 1 now the host > >> controller is assumed to be cache coherent. > >>> + rc->ats_attr = 1; > >> no ATS support instead? > >>> + rc->pci_seg_num = 0; > >> ID mappings are mandated for me to support MSIs with ITS. > >> > > > > These changes are made as I write, > > > > > >> Shannon told me we should match the kernel datatypes & fields > >> > >> for instance in include/acpi/actbl2.h we have: > >> > >> struct acpi_iort_id_mapping { > >> u32 input_base; /* Lowest value in input range */ > >> u32 id_count; /* Number of IDs */ > >> u32 output_base; /* Lowest value in output range */ > >> u32 output_reference; /* A reference to the output node */ > >> u32 flags; > >> }; > >> > >> This also holds for other struct definitions. > >> > >> > > Sure will change this accordingly. > > I currently have a series creating the IORT with an RC node and an ITS > node. It is needed to complete the integration of the virtual ITS (to > connect it with the PCI host controller). This originates from this > patch: I added the RC->ITS ID mapping + the ITS node and tested it. > > I don't know how to proceed to get the 2 features (vSMMU and vITS) > progress separately. Do you plan to respin this patch quickly? > Otherwise, if you are busy with other things I propose you to respin > fixing the few things above, splitting it into 3 patches, header [1], > ITS node creation [2], RC node creation with RC->ITS mapping [3] while > keeping credit to you on [1] and [3]. > > Then we can have a 4th patch adding RC-> SMMU ID > ITS mapping? > > Please let me know what are your plans and what do you think.
OK thanks. Looking forward to reviewing your patch Best Regards Eric > > Thanks > > Eric > > > > > > > Hi Eric, > I have been busy with something else, however I have a wokring patch set > (unclean version) > which creates the IORT tables, with SMMU->RC->ITS with ITS id mapping > (routing all interrupts to single ITS). > I'll push them by to unstable branch by this Sunday. > > -- > Cheers, > /Prem
