Re: [RFC PATCH 39/45] iommu/arm-smmu-v3-kvm: Initialize page table configuration

[Mostafa Saleh <smostafa@google.com>]

Hi Jean,

On Wed, Feb 01, 2023 at 12:53:23PM +0000, Jean-Philippe Brucker wrote:
> Prepare the stage-2 I/O page table configuration that will be used by
> the hypervisor driver.
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>  .../iommu/arm/arm-smmu-v3/arm-smmu-v3-kvm.c   | 29 +++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kvm.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kvm.c
> index 755c77bc0417..55489d56fb5b 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kvm.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kvm.c
> @@ -16,6 +16,7 @@ struct host_arm_smmu_device {
>  	struct arm_smmu_device		smmu;
>  	pkvm_handle_t			id;
>  	u32				boot_gbpa;
> +	unsigned int			pgd_order;
>  };
>  
>  #define smmu_to_host(_smmu) \
> @@ -192,6 +193,7 @@ static int kvm_arm_smmu_probe(struct platform_device *pdev)
>  	size_t size;
>  	phys_addr_t ioaddr;
>  	struct resource *res;
> +	struct io_pgtable_cfg cfg;
>  	struct arm_smmu_device *smmu;
>  	struct device *dev = &pdev->dev;
>  	struct host_arm_smmu_device *host_smmu;
> @@ -233,6 +235,31 @@ static int kvm_arm_smmu_probe(struct platform_device *pdev)
>  	if (!kvm_arm_smmu_validate_features(smmu))
>  		return -ENODEV;
>  
> +	/*
> +	 * Stage-1 should be easy to support, though we do need to allocate a
> +	 * context descriptor table.
> +	 */
> +	cfg = (struct io_pgtable_cfg) {
> +		.fmt = ARM_64_LPAE_S2,
> +		.pgsize_bitmap = smmu->pgsize_bitmap,
> +		.ias = smmu->ias,
> +		.oas = smmu->oas,
> +		.coherent_walk = smmu->features & ARM_SMMU_FEAT_COHERENCY,
> +	};
> +
> +	/*
> +	 * Choose the page and address size. Compute the PGD size and number of
> +	 * levels as well, so we know how much memory to pre-allocate.
> +	 */
> +	ret = io_pgtable_configure(&cfg, &size);
size variable is overwritten here with pgd size, while used later on
the assumption it still contains the SMMU MMIO size.
This looks like it is not intended?


> +	if (ret)
> +		return ret;
> +
> +	host_smmu->pgd_order = get_order(size);
> +	smmu->pgsize_bitmap = cfg.pgsize_bitmap;
> +	smmu->ias = cfg.ias;
> +	smmu->oas = cfg.oas;
> +
>  	ret = arm_smmu_init_one_queue(smmu, &smmu->cmdq.q, smmu->base,
>  				      ARM_SMMU_CMDQ_PROD, ARM_SMMU_CMDQ_CONS,
>  				      CMDQ_ENT_DWORDS, "cmdq");
> @@ -253,6 +280,8 @@ static int kvm_arm_smmu_probe(struct platform_device *pdev)
>  	hyp_smmu->mmio_addr = ioaddr;
>  	hyp_smmu->mmio_size = size;
>  	hyp_smmu->features = smmu->features;
> +	hyp_smmu->iommu.pgtable_cfg = cfg;
> +
>  	kvm_arm_smmu_cur++;
>  
>  	return 0;
> -- 
> 2.39.0
> 
Thanks,
Mostafa