On 10.09.19 00:24, Julien Grall wrote:
Hi Oleksandr,
Hi Julien
On 8/20/19 7:09 PM, Oleksandr Tyshchenko wrote:
diff --git a/xen/arch/arm/platforms/Kconfig
b/xen/arch/arm/platforms/Kconfig
index bc0e9cd..c93a6b2 100644
--- a/xen/arch/arm/platforms/Kconfig
+++ b/xen/arch/arm/platforms/Kconfig
@@ -25,6 +25,7 @@ config RCAR3
bool "Renesas RCar3 support"
depends on ARM_64
select HAS_SCIF
+ select IPMMU_VMSA
As discussed previously, I think the IPMMU driver should be merged as
tech preview for a couple of release to allow more users to test
before we mark it as supported.
Yes
Based on this, I would not advise to select IPMMU_VMSA by default as
user may not want to use tech preview code by default. Instead I would
only select if EXPERT is set.
Agree. Will do.
---help---
Enable all the required drivers for Renesas RCar3
diff --git a/xen/drivers/passthrough/Kconfig
b/xen/drivers/passthrough/Kconfig
index a3c0649..47eadb4 100644
--- a/xen/drivers/passthrough/Kconfig
+++ b/xen/drivers/passthrough/Kconfig
@@ -12,4 +12,17 @@ config ARM_SMMU
Say Y here if your SoC includes an IOMMU device
implementing the
ARM SMMU architecture.
+
+config IPMMU_VMSA
+ bool "Renesas IPMMU-VMSA found in R-Car Gen3 SoCs"
Shall I add EXPERT here also?
bool "Renesas IPMMU-VMSA found in R-Car Gen3 SoCs" if EXPERT = "y"
+ default n
+ depends on ARM_64
+ ---help---
+ Support for implementations of the Renesas IPMMU-VMSA found
+ in R-Car Gen3 SoCs.
+
+ Say Y here if you are using newest R-Car Gen3 SoCs revisions
+ (H3 ES3.0, M3-W+, etc) which IPMMU hardware supports stage 2
+ translation table format and is able to use CPU's P2M table as
is.
+
endif
[...]
+ /* Wait until the Root device has been registered for sure. */
+ if ( !mmu->root )
+ {
+ dev_err(&node->dev, "Root IPMMU hasn't been registered yet\n");
This is a bit odd to throw an error if we are going to defer the probe.
Agree. Will remove.
+static __init bool ipmmu_stage2_supported(void)
+{
+ struct dt_device_node *np;
+ uint64_t addr, size;
+ void __iomem *base;
+ uint32_t product, cut;
+ static enum
+ {
+ UNKNOWN,
+ SUPPORTED,
+ NOTSUPPORTED
+ } stage2_supported = UNKNOWN;
+
+ /* Use the flag to avoid checking for the compatibility more
then once. */
There are only one IOMMU root that will always be initialized first.
So can't you move this code in the root IOMMU path?
Actually, I can. Good point.
+ switch ( stage2_supported )
+ {
+ case SUPPORTED:
+ return true;
+
+ case NOTSUPPORTED:
+ return false;
+
+ case UNKNOWN:
+ default:
+ stage2_supported = NOTSUPPORTED;
+ break;
+ }
+
+ np = dt_find_compatible_node(NULL, NULL, "renesas,prr");
+ if ( !np )
+ {
+ printk(XENLOG_ERR "ipmmu: Failed to find PRR node\n");
+ return false;
+ }
+
+ if ( dt_device_get_address(np, 0, &addr, &size) )
+ {
+ printk(XENLOG_ERR "ipmmu: Failed to get PRR MMIO\n");
+ return false;
+ }
+
+ base = ioremap_nocache(addr, size);
+ if ( !base )
+ {
+ printk(XENLOG_ERR "ipmmu: Failed to ioremap PRR MMIO\n");
+ return false;
+ }
+
+ product = readl(base);
+ cut = product & RCAR_CUT_MASK;
+ product &= RCAR_PRODUCT_MASK;
+
+ switch ( product )
+ {
+ case RCAR_PRODUCT_H3:
+ case RCAR_PRODUCT_M3W:
+ if ( cut >= RCAR_CUT_VER30 )
+ stage2_supported = SUPPORTED;
+ break;
+
+ case RCAR_PRODUCT_M3N:
+ stage2_supported = SUPPORTED;
+ break;
+
+ default:
+ printk(XENLOG_ERR "ipmmu: Unsupported SoC version\n");
+ break;
+ }
+
+ iounmap(base);
+
+ return stage2_supported == SUPPORTED;
+}
+
+static const struct dt_device_match ipmmu_dt_match[] __initconst =
+{
+ DT_MATCH_COMPATIBLE("renesas,ipmmu-r8a7795"),
+ DT_MATCH_COMPATIBLE("renesas,ipmmu-r8a77965"),
+ DT_MATCH_COMPATIBLE("renesas,ipmmu-r8a7796"),
+ { /* sentinel */ },
+};
+
+static __init int ipmmu_init(struct dt_device_node *node, const void
*data)
+{
+ int ret;
+
+ /*
+ * Even if the device can't be initialized, we don't want to give
+ * the IPMMU device to dom0.
+ */
+ dt_device_set_used_by(node, DOMID_XEN);
+
+ if ( !iommu_hap_pt_share )
iommu_hap_pt_share will soon be hardcoded to true on Arm (see [1]).
Even without the patch, the value of iommu_hap_pt_share should be
ignored as done by the SMMU.
I got it. Will ignore.
+ {
+ printk_once(XENLOG_ERR "ipmmu: P2M table must always be
shared between the CPU and the IPMMU\n");
+ return -EINVAL;
+ }
+
+ if ( !ipmmu_stage2_supported() )
+ {
+ printk_once(XENLOG_ERR "ipmmu: P2M sharing is not supported
in current SoC revision\n");
+ return -ENODEV;
The if part is returning so...
+ }
+ else
... the else part is not necessary removing one layer of indentation.
ok
--
Regards,
Oleksandr Tyshchenko
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
