Re: [XEN PATCH v3 3/3] xen/mm: add declaration for first_valid_mfn

[Julien Grall <julien@xxxxxxx>]

Hi,

On 15/12/2023 09:59, Nicola Vetrini wrote:
> > > > +extern mfn_t first_valid_mfn;
> > > > +
> > > >    /*
> > > >     * Create only non-leaf page table entries for the
> > > >     * page range in Xen virtual address space.
> > > ... I still disagree with the placement here (should be xen/numa.h imo),
> > I don't care too much about which header is used. So I am ok with 
> > numa.h if this is what you really want. However...
> > > and I still don't see why we can't carry out the TODO right away, if we
> > > have to touch all of this anyway. If it's really too much to ask from
> > > the original contributor, I can certainly see about making a patch 
> > > myself
> > There are a few reasons I didn't update with that approach or want to 
> > ask Nicola for this:
> >   1. I assume this will be a macro and it is not clear to me whether 
> > Eclair would be able to detect the visibility change based on the config
> >   2. IMHO, this is obfuscating a bit more the code
> >   3. I don't see why we want to special case first_valid_mfn. I am 
> > sure we have other external variables in the same situation.
> > So if any solution needs to came up, then it needs to be generic, and 
> > I fear this is going to be yet another lengthy work which is not worth 
> > it for this case.
> > > (and I've now added this to my short-term TODO list).
> > Thanks. But as I wrote above, I am not sure I agree such proposal. Let 
> > see what the code looks like.
> Given the controversy that arose on this patch, maybe it's best to drop 
> it now from this series. If then it's agreed that as a temporary 
> workaround you want to leave it as is and deviate, as I proposed 
> elsewhere in the thread, feel free to reach out. Meanwhile, if patch 1 
> looks ok I can mark this series as done and focus on the v2 for R2.1, 
> which is almost ready.
I am still not in favor of deviation. We would need to create a new 
SAF-* (because none of the existing one are not correct) and it is not 
clear how you could argue this is a good deviation.
Just to be clear, this is not a Nack for a deviation. I find just silly 
we are trying to use deviation here just because there is a small roadblock.
To me the goal is to try to improve Xen, not trying to deviate 
everything because this is easier.
Cheers,

--
Julien Grall