Hi Manish,
On 18/01/18 06:15, mja...@caviumnetworks.com wrote:
From: Manish Jaggi <mja...@caviumnetworks.com>
This patch introduces a function vgic_map_translation_space for mapping
ITS 64K translater space for doorbells in dom0 stage-2.
It is required as dom0 PCI devices behined SMMU wont be able to write MSIs.
s/behined/behind/
also there a two spaces after SMMU. One should be enough.
This function is called from vgic_v3_its_init_domain only for hardware domain.
This is really confusing. You mix dom0 and hardware domain in the same
commit. Please use only "hardware domain".
This patch is also required for testing
[RFC 00/11] acpi: arm: IORT Support for Xen [1]
+ [2] [RFC v4 0/8] SMMUv3 driver
[1] https://lists.xenproject.org/archives/html/xen-devel/2018-01/msg00007.html
[2] https://lists.xen.org/archives/html/xen-devel/2017-12/msg01294.html
Anything after "This patch..." does not seem to be relevant in the
commit. You might want to add that after --- so it get stripped after
committing.
Signed-off-by: Manish Jaggi <manish.ja...@cavium.com>
---
xen/arch/arm/vgic-v3-its.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c
index 72a5c70656..a8f8079149 100644
--- a/xen/arch/arm/vgic-v3-its.c
+++ b/xen/arch/arm/vgic-v3-its.c
@@ -1511,6 +1511,30 @@ unsigned int vgic_v3_its_count(const struct domain *d)
return ret;
}
+static int vgic_map_translation_space(struct domain *d, paddr_t guest_addr)
This function is quite confusing. What is guest_addr? It looks like it
is the base address of the ITS but it is not very clear here.
+{
+ u64 addr, size;
This should be paddr_t.
+ int ret;
Newline here please.
+ addr = guest_addr + SZ_64K;
+ size = SZ_64K;
+
+ ret = map_mmio_regions(d,
+ paddr_to_pfn(addr & PAGE_MASK),
+ DIV_ROUND_UP(size, PAGE_SIZE),
+ paddr_to_pfn(addr & PAGE_MASK));
You are assuming a 1:1 mapping in the domain. Nowhere you explain that
assumption nor that it will not work for guest. At least you need an
ASSERT(is_domain_direct_mapped(d)).
Furthermore, as we discussed yesterday I would expect a big fat comment
that this need to be revisited when DomU support will be added as it may
not be safe to map the Doorbell in page-tables used by the processor.
+
+ if ( ret )
+ {
+ printk(XENLOG_ERR "Unable to map to dom%d access to"
+ " 0x%"PRIx64" - 0x%"PRIx64"\n",
+ d->domain_id,
+ addr & PAGE_MASK, PAGE_ALIGN(addr + size) - 1);
+ }
+
+ return ret;
+
+}
+
/*
* For a hardware domain, this will iterate over the host ITSes
* and map one virtual ITS per host ITS at the same address.
@@ -1541,6 +1565,8 @@ int vgic_v3_its_init_domain(struct domain *d)
return ret;
else
d->arch.vgic.has_its = true;
+
+ vgic_map_translation_space(d, hw_its->addr);
Your new function may return an error. Therefore you should test it.
}
}
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
