Re: [PATCH v3 3/3] xen/iommu: arm: Use p2m_ipa_bits as stage2 input size

[Julien Grall <julien.grall@citrix.com>]

Hi Edgar,

On 30/04/15 12:55, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> 
> 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 <edgar.iglesias@xilinx.com>
> ---
>  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