Re: [Xen-devel] [PATCH V4 8/8] iommu/arm: Add Renesas IPMMU-VMSA support

[Oleksandr <olekstysh@xxxxxxxxx>]

On 19.09.19 14:45, Julien Grall wrote:
> Hi Oleksandr,

Hi Julien


> On 13/09/2019 16:35, Oleksandr Tyshchenko wrote:
> > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> >
> > The IPMMU-VMSA is VMSA-compatible I/O Memory Management Unit (IOMMU)
> > which provides address translation and access protection functionalities
> > to processing units and interconnect networks.
> >
> > Please note, current driver is supposed to work only with newest
> > R-Car Gen3 SoCs revisions which IPMMU hardware supports stage 2 
> > translation
> > table format and is able to use CPU's P2M table as is if one is
> > 3-level page table (up to 40 bit IPA).
> >
> > The major differences compare to the Linux driver are:
> >
> > 1. Stage 1/Stage 2 translation. Linux driver supports Stage 1
> > translation only (with Stage 1 translation table format). It manages
> > page table by itself. But Xen driver supports Stage 2 translation
> > (with Stage 2 translation table format) to be able to share the P2M
> > with the CPU. Stage 1 translation is always bypassed in Xen driver.
> >
> > So, Xen driver is supposed to be used with newest R-Car Gen3 SoC 
> > revisions
> > only (H3 ES3.0, M3-W+, etc.) which IPMMU H/W supports stage 2 
> > translation
> > table format.
> >
> > 2. AArch64 support. Linux driver uses VMSAv8-32 mode, while Xen driver
> > enables Armv8 VMSAv8-64 mode to cover up to 40 bit input address.
> >
> > 3. Context bank (sets of page table) usage. In Xen, each context bank is
> > mapped to one Xen domain. So, all devices being pass throughed to the
> > same Xen domain share the same context bank.
> >
> > 4. IPMMU device tracking. In Xen, all IOMMU devices are managed
> > by single driver instance. So, driver uses global list to keep track
> > of registered IPMMU devices.
> >
> > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> > CC: Julien Grall <julien.grall@xxxxxxx>
> > CC: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>
> The Xen specific bits looks good now. Assuming Yoshihiro will give his 
> reviewed-by/acked-by:
> Acked-by: Julien Grall <julien.grall@xxxxxxx>
Thank you!


One note, it turned out [1] that "args" parameter in "dt_xlate" callback 
needs "const" added (it is not assumed to modify it inside the callback).
This leads to additional changes to the IPMMU driver. If you happy with 
the changes below, I will retain your A-b.

diff --git a/xen/drivers/passthrough/arm/ipmmu-vmsa.c 
b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
index b5c18c2..43e4a84 100644
--- a/xen/drivers/passthrough/arm/ipmmu-vmsa.c
+++ b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
@@ -696,7 +696,7 @@ static void ipmmu_detach_device(struct 
ipmmu_vmsa_domain *domain,
 }

 static int ipmmu_init_platform_device(struct device *dev,
-                                      struct dt_phandle_args *args)
+                                      const struct dt_phandle_args *args)
 {
     struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
     struct ipmmu_vmsa_device *mmu;
@@ -1174,7 +1174,8 @@ static int ipmmu_reassign_device(struct domain *s, 
struct domain *t,
     return 0;
 }

-static int ipmmu_dt_xlate(struct device *dev, struct dt_phandle_args *spec)
+static int ipmmu_dt_xlate(struct device *dev,
+                          const struct dt_phandle_args *spec)
 {
     int ret;

@@ -1187,7 +1188,7 @@ static int ipmmu_dt_xlate(struct device *dev, 
struct dt_phandle_args *spec)
     if ( spec->args_count != 1 || spec->args[0] >= IPMMU_UTLB_MAX )
         return -EINVAL;

-    ret = iommu_fwspec_add_ids(dev, spec->args, 1);
+    ret = iommu_fwspec_add_ids(dev, (uint32_t *)spec->args, 1);
     if ( ret )
         return ret;


[1] 
https://lists.xenproject.org/archives/html/xen-devel/2019-09/msg01443.html

--
Regards,

Oleksandr Tyshchenko


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel