On Fri, Sep 9, 2016 at 8:54 PM, Auger Eric <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>
> > ---
> > 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.
--
Cheers,
/Prem
