Re: [PATCH v2 3/3] x86/PVH: Support relocatable dom0 kernels

[Jason Andryuk <jason.andryuk@xxxxxxx>]

On 2024-03-14 11:30, Jan Beulich wrote:
> On 14.03.2024 15:33, Roger Pau Monné wrote:
> > On Thu, Mar 14, 2024 at 09:51:22AM -0400, Jason Andryuk wrote:
> > > On 2024-03-14 05:48, Roger Pau Monné wrote:
> > > > On Wed, Mar 13, 2024 at 03:30:21PM -0400, Jason Andryuk wrote:
> > > > > @@ -234,6 +235,17 @@ elf_errorstatus elf_xen_parse_note(struct elf_binary *elf,
> > > > >                    elf_note_numeric_array(elf, note, 8, 0),
> > > > >                    elf_note_numeric_array(elf, note, 8, 1));
> > > > >            break;
> > > > > +
> > > > > +    case XEN_ELFNOTE_PVH_RELOCATION:
> > > > > +        if ( elf_uval(elf, note, descsz) != 3 * sizeof(uint64_t) )
> > > > > +            return -1;
> > > > > +
> > > > > +        parms->phys_min = elf_note_numeric_array(elf, note, 8, 0);
> > > > > +        parms->phys_max = elf_note_numeric_array(elf, note, 8, 1);
> > > > > +        parms->phys_align = elf_note_numeric_array(elf, note, 8, 2);
> > > > Size for those needs to be 4 (32bits) as the entry point is in 32bit
> > > > mode?  I don't see how we can start past the 4GB boundary.
> > > I specified the note as 3x 64bit values.  It seemed simpler than trying to
> > > support both 32bit and 64bit depending on the kernel arch.  Also, just using
> > > 64bit provides room in case it is needed in the future.
> > Why do you say depending on the kernel arch?
> >
> > PVH doesn't know the bitness of the kernel, as the kernel entry point
> > is always started in protected 32bit mode.  We should just support
> > 32bit values, regardless of the kernel bitness, because that's the
> > only range that's suitable in order to jump into the entry point.
> >
> > Note how XEN_ELFNOTE_PHYS32_ENTRY is also unconditionally a 32bit
> > integer.
Linux defines PHYS32_ENTRY with _ASM_PTR, so it's 32 or 64 bit to match 
how the kernel is compiled.  The Xen code parses the integer according 
to the size of the note.
> > > Do you want the note to be changed to 3x 32bit values?
> > Unless anyone objects, yes, that's would be my preference.
> As mentioned elsewhere, unless the entire note is meant to be x86-specific,
> this fixed-32-bit property then would want limiting to x86.
PVH is only implemented for x86 today.  Are you saying that the comment 
should just specify the values are 32bit for x86?  If the note is reused 
for other arches, then they can specify their usage?
If this note is to be a variably sized array of values, then the 
elements should be of fixed size.  Otherwise parsing is ambiguous 
without, say, another field specifying element size.
Maybe XEN_ELFNOTE_PHYS32_RELOC would be a better name to complement the 
PHYS32_ENTRY?
Regards,
Jason