[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] x86/iommu: fix wrong iterator type in arch_iommu_hwdom_init()
- To: Roger Pau Monne <roger.pau@xxxxxxxxxx>
- From: Jan Beulich <jbeulich@xxxxxxxx>
- Date: Tue, 9 May 2023 18:25:05 +0200
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=gE5Ni0+G6ask8+koWbUDzG5lMKS5+OBPt48hbBBuaYY=; b=Dgz7yZAnJmHFPto3ewne9U2p8DKmNCoUBVuqcaXiqvJkVepy5cdsyF+MvrDxDgJBiPBNTSTAt43ToMBRcZpWGqnNbFHlKfa7qy2KJVrR5YuPTORguvLwPVPj47VgHN0PR7eDchQmn5vO9meI3v2J+WPw0UpaVzfUM6szO7NBHXJYC5aakMKpR21Noyy9N/vxSZCkUtNcaYCGn6t5i1a4gelVy4rmICl9eIQwZkWVQWspNZVWdS1Hk2LXEJgIq7egy/slXGrTu4/qKEoIJ16hvNwKbNixiUeuxu1SjWihZztX6/u0C9lp300srVIEAL+arYFX8Te0J6ZWUiVYW3x5lg==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=I/lCRKkVFlvsenPzdEq2mi7nG738Lo2D2Rsf9rs63V8UjFl5ah0E5JJbWaZkBf1P1aWszPeKMAxj33+5SwWVOmDc2JONmW6aL/RQzkOhWbyRmvfdaoVsEsBM8Ha8L3j+wHpOm4FIAkngXThxd6hKfNbMnxqWCocmjBPC4l3NB/NYk2UT7mOJ9wtyJ12dAb9QeC+oDW/AlvIDMuwQCTSNHKCdTLd1npKONfIp5zryCnXmvuy+Sy4cLpYI/q8ehi/v7qhEQgDIIekYCJZ+AC2UI+dnjlnKbS5O6MK+KMQOZDciuRCtsMUyWR7xxiJyO3ZGOo+Q976MybVtN+S37qiDwQ==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
- Cc: Paul Durrant <paul@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
- Delivery-date: Tue, 09 May 2023 16:25:24 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 09.05.2023 13:03, Roger Pau Monne wrote:
> The 'i' iterator index stores a pdx, not a pfn, and hence the initial
> assignation of start (which stores a pfn) needs a conversion from pfn
> to pdx.
Strictly speaking: Yes. But pdx compression skips the bottom MAX_ORDER
bits, so ...
> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -406,7 +406,7 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
> */
> start = paging_mode_translate(d) ? PFN_DOWN(MB(1)) : 0;
... with this, ...
> - for ( i = start, count = 0; i < top; )
> + for ( i = pfn_to_pdx(start), count = 0; i < top; )
... this is an expensive identity transformation. Could I talk you into
adding
ASSERT(start == pfn_to_pdx(start));
instead (or the corresponding BUG_ON() if you'd prefer that, albeit then
the expensive identity transformation will still be there even in release
builds; not that it matters all that much right here, but still)?
In any event, with no real bug fixed (unless I'm overlooking something),
I would suggest to drop the Fixes: tag.
Jan
|
