[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241031125306.GF10193@nvidia.com>
Date: Thu, 31 Oct 2024 09:53:06 -0300
From: Jason Gunthorpe <jgg@...dia.com>
To: Suravee Suthikulpanit <suravee.suthikulpanit@....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 v7 06/10] iommu/amd: Modify set_dte_entry() to use
256-bit DTE helpers
On Thu, Oct 31, 2024 at 09:16:20AM +0000, Suravee Suthikulpanit wrote:
> @@ -1148,6 +1146,28 @@ static bool copy_device_table(void)
> return true;
> }
>
> +struct dev_table_entry *amd_iommu_get_ivhd_dte_flags(u16 devid)
> +{
> + u16 f = 0, l = 0xFFFF;
> + struct ivhd_dte_flags *e;
> + struct dev_table_entry *dte = NULL;
> +
> + for_each_ivhd_dte_flags(e) {
Maybe the list head should be on the iommu? I don't know how long it
would be if it is worthwhile.
> + /*
> + * Need to go through the whole list to find the smallest range,
> + * which contains the devid. Then store it in f and l variables.
> + */
> + if ((e->devid_first >= devid) && (e->devid_last <= devid)) {
> + if (f < e->devid_first)
> + f = e->devid_first;
> + if (e->devid_last < l)
> + l = e->devid_last;
> + dte = &(e->dte);
> + }
f and l are never used, why calculate them?
Isn't (e->devid_first >= devid) not the right way to check if devid
falls within a range?
Based on the comment it seems like you want something like this??
struct dev_table_entry *amd_iommu_get_ivhd_dte_flags(u16 devid)
{
struct dev_table_entry *dte = NULL;
unsigned int best_len = UINT_MAX;
struct ivhd_dte_flags *e;
for_each_ivhd_dte_flags(e) {
/*
* Need to go through the whole list to find the smallest range,
* which contains the devid. Then store it in f and l variables.
*/
if ((e->devid_first <= devid) && (e->devid_last >= devid)) {
unsigned int len = e->devid_last - e->devid_first;
if (len < best_len) {
dte = &(e->dte);
best_len = len;
}
}
}
return dte;
}
(and it would be smart to sort the linked list by size, but again I
don't know how big it is if it is worthwile complexity)
The DTE code looks OK
Jason
Powered by blists - more mailing lists