>>> On 12.10.18 at 19:18, <[email protected]> wrote: >> -----Original Message----- >> From: Woods, Brian [mailto:[email protected]] >> Sent: 12 October 2018 18:14 >> To: Paul Durrant <[email protected]> >> Cc: [email protected]; Suthikulpanit, Suravee >> <[email protected]>; Woods, Brian <[email protected]> >> Subject: Re: [PATCH] amd-iommu: get rid of pointless >> IOMMU_PAGING_MODE_LEVEL_X definitions >> >> On Thu, Sep 27, 2018 at 01:46:01PM +0100, Paul Durrant wrote: >> > The levels are absolute numbers such that IOMMU_PAGING_MODE_LEVEL_X >> > evaluates to X (for the valid range of 0 - 7) so simply use numbers in >> > the code. >> > >> > No functional change. >> > >> > NOTE: This patch also adds emacs boilerplate to amd-iommu-defs.h >> > >> > Signed-off-by: Paul Durrant <[email protected]> >> >> Is there a strong reason to get rid of these? Some of examples below >> create seemingly magic numbers in the code. While if you're familiar >> with the functions this isn't a big deal, otherwise you have to dig >> further to tell. >> > > The numbers aren't magic though. The spec refers to levels by number rather > than any sort of name. If the levels were named then it would be absolutely > right to #define <level name> <level number>, but that is not the case. Thus > IMO > getting rid of the definitions actually makes the code clearer for those > (like myself) reading the spec. > >> > + pte = table + pfn_to_pde_idx(gfn, 1); >> >> > + need_flush = set_iommu_pde_present(pde, next_mfn, 0, iw, ir); >> >> If there's a general consensus that getting rid of these is better, I >> don't mind and will agree to it. >> > > Anyone else care to comment?
I think that, quite the opposite of what is often the case, the amount of manifest constants the AMD IOMMU code uses is quite a bit too large. I therefore welcome this change, and I've been planning some other reduction there (but haven't got to it in a couple of years). Jan _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
