[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



On Thu, Mar 14, 2024 at 12:59:19PM -0400, Jason Andryuk wrote:
> 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.

I think that's wrong, PHYS32_ENTRY should strictly be a 32bit integer,
in fact the field in struct elf_dom_parms is an uint32_t, so Linux
using _ASM_PTR seems bogus, it should unconditionally use .long
regardless of the kernel bitness.

> > > > 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?

IMO the '32' part of PHYS32_ENTRY is kind of redundant, given the CPU
state when using such entry point it's impossible to use 64bit
addresses.  I would be fine with using XEN_ELFNOTE_PHYS_RELOC or some
such.  Anyway, this is just a name so I'm not going to opposed if Jan
and yourself prefer to keep using PHYS32.

Thanks, Roger.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.