> -----Original Message----- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 25 October 2018 11:29 > To: Brian Woods <brian.wo...@amd.com>; Paul Durrant > <paul.durr...@citrix.com> > Cc: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>; xen-devel <xen- > de...@lists.xenproject.org> > Subject: Re: [Xen-devel] [PATCH] amd-iommu: get rid of pointless > IOMMU_PAGING_MODE_LEVEL_X definitions > > >>> On 12.10.18 at 19:18, <paul.durr...@citrix.com> wrote: > >> -----Original Message----- > >> From: Woods, Brian [mailto:brian.wo...@amd.com] > >> Sent: 12 October 2018 18:14 > >> To: Paul Durrant <paul.durr...@citrix.com> > >> Cc: xen-devel@lists.xenproject.org; Suthikulpanit, Suravee > >> <suravee.suthikulpa...@amd.com>; Woods, Brian <brian.wo...@amd.com> > >> 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 <paul.durr...@citrix.com> > >> > >> 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). >
Brian, Are you ok to ack this patch now, or would you like more opinions? Paul _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
