From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v3 3/3] xen/iommu: arm: Use p2m_ipa_bits as stage2 input size Date: Thu, 30 Apr 2015 16:07:27 +0100 Message-ID: <5542452F.4010404@citrix.com> References: <1430394937-29122-1-git-send-email-edgar.iglesias@gmail.com> <1430394937-29122-4-git-send-email-edgar.iglesias@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1430394937-29122-4-git-send-email-edgar.iglesias@gmail.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "Edgar E. Iglesias" , xen-devel@lists.xen.org Cc: julien.grall@citrix.com, edgar.iglesias@xilinx.com, tim@xen.org, ian.campbell@citrix.com, stefano.stabellini@citrix.com List-Id: xen-devel@lists.xenproject.org Hi Edgar, On 30/04/15 12:55, Edgar E. Iglesias wrote: > From: "Edgar E. Iglesias" > > The Stage2 input-size must match what the CPU uses because > the SMMU and the CPU share page-tables. > > Assert that the SMMU supports the P2M IPA bit size and use it. > > Signed-off-by: Edgar E. Iglesias > --- > xen/drivers/passthrough/arm/smmu.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c > index 8a9b58b..5356046 100644 > --- a/xen/drivers/passthrough/arm/smmu.c > +++ b/xen/drivers/passthrough/arm/smmu.c > @@ -2230,8 +2230,9 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu) > size = arm_smmu_id_size_to_bits((id >> ID2_IAS_SHIFT) & ID2_IAS_MASK); > smmu->s1_output_size = min_t(unsigned long, PHYS_MASK_SHIFT, size); > > - /* Xen: Stage-2 input size is not restricted */ > - smmu->s2_input_size = size; > + /* Xen: Stage-2 input size has to match p2m_ipa_bits. */ > + ASSERT(size >= p2m_ipa_bits); Sorry, I was planning to comment on this patch in the previous version later but you send the new version quickly. In general, ASSERT should only be used when you think that the condition is always true. Although, we have some hardware where we know that this condition will be hit. On non-debug, we would get an odd error later because ASSERT is turned into a no-op. As this is a restriction of the driver we should print a error message and return an appropriate error value. The generic IOMMU driver can then decide if it's safe to continue without the SMMU setup or panic. FWIW, currently we use the later. I will send a patch to panic avoiding the user to think the SMMU is correctly setup. Regards, -- Julien Grall