Re: [PATCH v3 3/4] mini-os: fix number of pages for PVH

[Juergen Gross <jgross@xxxxxxxx>]

On 21.06.22 10:43, Andrew Cooper wrote:
> On 21/06/2022 08:23, Juergen Gross wrote:
> > diff --git a/e820.c b/e820.c
> > index ad91e00b..49b16878 100644
> > --- a/e820.c
> > +++ b/e820.c
> > @@ -29,6 +29,38 @@
> >   #include <mini-os/e820.h>
> >   #include <xen/memory.h>
> >   
> > +static unsigned long e820_initial_reserved_pfns;
> > +
> > +unsigned long e820_get_current_pages(void)
> > +{
> > +    domid_t domid = DOMID_SELF;
> > +    long ret;
> > +
> > +    ret = HYPERVISOR_memory_op(XENMEM_current_reservation, &domid);
> > +    if ( ret < 0 )
> > +    {
> > +        xprintk("could not get memory size\n");
> %ld ret
>
> Also, xprintk() vs ...
>
> > +        do_exit();
> > +    }
> > +
> > +    return ret - e820_initial_reserved_pfns;
> > +}
> > +
> > +unsigned long e820_get_max_pages(void)
> > +{
> > +    domid_t domid = DOMID_SELF;
> > +    long ret;
> > +
> > +    ret = HYPERVISOR_memory_op(XENMEM_maximum_reservation, &domid);
> > +    if ( ret < 0 )
> > +    {
> > +        printk("Could not get maximum pfn\n");
> ... printk()?
>
> Shouldn't they both be printk()?  Can fix both issues on commit.
e820_get_current_pages() is being called before console initialization,
so it should really use xprintk().

Adding the returned error might be interesting, though (even if an error
is very unlikely).

Note that the error message printing has just been moved from the previous
use case into the new function.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature