linux-kernel - Re: [PATCH RFC RESEND 4/6] iommu/riscv: support nested iommu for getting iommu hardware information

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 7 May 2024 23:19:58 +0800
From: Zong Li <zong.li@...ive.com>
To: Jason Gunthorpe <jgg@...pe.ca>
Cc: joro@...tes.org, will@...nel.org, robin.murphy@....com, 
	tjeznach@...osinc.com, paul.walmsley@...ive.com, palmer@...belt.com, 
	aou@...s.berkeley.edu, kevin.tian@...el.com, linux-kernel@...r.kernel.org, 
	iommu@...ts.linux.dev, linux-riscv@...ts.infradead.org
Subject: Re: [PATCH RFC RESEND 4/6] iommu/riscv: support nested iommu for
 getting iommu hardware information

On Tue, May 7, 2024 at 10:54 PM Jason Gunthorpe <jgg@...pe.ca> wrote:
>
> On Tue, May 07, 2024 at 10:25:58PM +0800, Zong Li wrote:
> > +{
> > +     struct riscv_iommu_device *iommu = dev_to_iommu(dev);
> > +     struct iommu_hw_info_riscv_iommu *info;
> > +
> > +     if (!iommu)
> > +             return ERR_PTR(-ENODEV);
>
> This is not possible, don't include impossible checks like this.

Thanks for pointing this out, I will remove it in the next version.

>
> > +     info = kzalloc(sizeof(*info), GFP_KERNEL);
> > +     if (!info)
> > +             return ERR_PTR(-ENOMEM);
> > +
> > +     info->capability = iommu->caps;
> > +     info->fctl = riscv_iommu_readl(iommu, RISCV_IOMMU_REG_FCTL);
> > +
> > +     *length = sizeof(*info);
> > +     *type = IOMMU_HW_INFO_TYPE_RISCV_IOMMU;
> > +
> > +     return info;
> > +}
> > +
> >  static int riscv_iommu_device_domain_type(struct device *dev)
> >  {
> >       return 0;
> > @@ -1560,6 +1582,7 @@ static void riscv_iommu_release_device(struct device *dev)
> >  static const struct iommu_ops riscv_iommu_ops = {
> >       .pgsize_bitmap = SZ_4K,
> >       .of_xlate = riscv_iommu_of_xlate,
> > +     .hw_info = riscv_iommu_hw_info,
> >       .identity_domain = &riscv_iommu_identity_domain,
> >       .blocked_domain = &riscv_iommu_blocking_domain,
> >       .release_domain = &riscv_iommu_blocking_domain,
> > diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h
> > index 1dfeaa2e649e..ec9aafd7d373 100644
> > --- a/include/uapi/linux/iommufd.h
> > +++ b/include/uapi/linux/iommufd.h
> > @@ -475,15 +475,28 @@ struct iommu_hw_info_vtd {
> >       __aligned_u64 ecap_reg;
> >  };
> >
> > +/**
> > + * struct iommu_hw_info_riscv_iommu - RISCV IOMMU hardware information
> > + *
> > + * @capability: Value of RISC-V IOMMU capability register
> > + * @fctl: Value of RISC-V IOMMU feature control register
> > + */
>
> Please call out explictly what spec these values come from.

Let me add the description for the section that defines them.

>
> > +struct iommu_hw_info_riscv_iommu {
> > +     __aligned_u64 capability;
> > +     __u32 fctl;
> > +};
>
> Add explicit padding here

Add a u32 reserve here in the next version. Thanks

>
> Jason