Re: [PATCH 05/17] xen/x86: Remove the non-typesafe version of pagetable_* helpers

[Julien Grall <julien@xxxxxxx>]

Hi,

On 20/04/2020 10:16, Jan Beulich wrote:
> On 18.04.2020 12:23, Julien Grall wrote:
> > On 30/03/2020 08:52, Jan Beulich wrote:
> > > On 28.03.2020 11:52, Julien Grall wrote:
> > > > On 26/03/2020 15:39, Jan Beulich wrote:
> > > > > On 22.03.2020 17:14, julien@xxxxxxx wrote:
> > > > > > @@ -3116,24 +3116,24 @@ int vcpu_destroy_pagetables(struct vcpu *v)
> > > > > >           /* Free that page if non-zero */
> > > > > >         do {
> > > > > > -        if ( mfn )
> > > > > > +        if ( !mfn_eq(mfn, _mfn(0)) )
> > > > > I admit I'm not fully certain either, but at the first glance
> > > > >
> > > > >            if ( mfn_x(mfn) )
> > > > >
> > > > > would seem more in line with the original code to me (and then
> > > > > also elsewhere).
> > > > It is doing *exactly* the same things. The whole point of typesafe
> > > > is to use typesafe helper not open-coding test everywhere.
> > > >
> > > > It is also easier to spot any use of MFN 0 within the code as you
> > > > know could grep "_mfn(0)".
> > > >
> > > > Therefore I will insist to the code as-is.
> > > What I insit on is that readability of the result of such changes be
> > > also kept in mind. The mfn_eq() construct is (I think) clearly less
> > > easy to read and recognize than the simpler alternative suggested.
> > If mfn_eq() is less clear, then where do you draw the line when the
> > macro should or not be used?
> I'm afraid there may not be a clear line to draw until everything
> got converted.
I am sorry but this doesn't add up. Here you say that we can't have a 
clear line to draw until everything is converted but...
> I do seem to recall though that, perhaps in a
> different context, Andrew recently agreed with my view here (Andrew,
> please correct me if I'm wrong). It being a fuzzy thing, I guess
> maintainers get to judge ...
... here you say the maintainers get to decide when to use mfn_eq() (or 
other typesafe construction). So basically, we would never be able to 
fully convert the code and therefore never draw a line.
As I am trying to convert x86 to use typesafe, I would like a bit more 
guidelines on your expectation for typesafe. Can you clarify it?
Cheers,

--
Julien Grall