linux-kernel - Re: [PATCH v4 5/6] iommu/amd: Modify clear_dte

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]

Message-ID: <f2a87143-5879-4ec9-ab3b-b2a6df12566e@amd.com>
Date: Thu, 3 Oct 2024 23:15:59 +0700
From: "Suthikulpanit, Suravee" <suravee.suthikulpanit@....com>
To: Jason Gunthorpe <jgg@...dia.com>
Cc: linux-kernel@...r.kernel.org, iommu@...ts.linux.dev, joro@...tes.org,
 robin.murphy@....com, vasant.hegde@....com, kevin.tian@...el.com,
 jon.grimm@....com, santosh.shukla@....com, pandoh@...gle.com,
 kumaranand@...gle.com
Subject: Re: [PATCH v4 5/6] iommu/amd: Modify clear_dte_entry() to avoid
 in-place update

On 9/27/2024 2:54 AM, Jason Gunthorpe wrote:
> On Mon, Sep 16, 2024 at 05:18:04PM +0000, Suravee Suthikulpanit wrote:
>
> ....
>
> I suggest you change this slightly so the flow is more like
> 
> make_clear_dte(..., struct dev_table_entry *entry) {..}
> 
> Which would have most of the above. Then:
> 
> clear_dte_entry()
> {
>      struct dev_table_entry target;
> 
>      make_clear_dte(.., &target);
>      update_dte256(iommu, dev_data, &new);
> }
> 
> And then in the prior patches you can write like:
> 
> static void make_dte_gcr3_table(struct amd_iommu *iommu,
>                                struct iommu_dev_data *dev_data,
>                                struct dev_table_entry *target)
> {
>      make_clear_dte(.., &target);
>      ...
> }
> 
> And drop all the wild masking:
> 
> +       /* First mask out possible old values for GCR3 table */
> +       tmp = DTE_GCR3_VAL_A(~0ULL) << DTE_GCR3_SHIFT_A;
> +       target->data[0] &= ~tmp;
> +       tmp = DTE_GCR3_VAL_B(~0ULL) << DTE_GCR3_SHIFT_B;
> +       tmp |= DTE_GCR3_VAL_C(~0ULL) << DTE_GCR3_SHIFT_C;
> +       target->data[1] &= ~tmp;
> 
> Since make_clear_dte() already zero'd these fields.

Thanks for suggestion. I'll clean this up.

Suravee