Re: [Xen-devel] [PATCH v2] x86/pvh: copy data from low 1MB to Dom0 physmap instead of mapping it

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.09.18 at 09:14, <roger.pau@xxxxxxxxxx> wrote:
> On Tue, Sep 18, 2018 at 07:10:31AM -0600, Jan Beulich wrote:
>> >>> On 18.09.18 at 10:55, <roger.pau@xxxxxxxxxx> wrote:
>> > @@ -420,16 +394,27 @@ static int __init pvh_setup_p2m(struct domain *d)
>> >          addr = PFN_DOWN(d->arch.e820[i].addr);
>> >          size = PFN_DOWN(d->arch.e820[i].size);
>> >  
>> > -        if ( addr >= MB1_PAGES )
>> > -            rc = pvh_populate_memory_range(d, addr, size);
>> > -        else
>> > -        {
>> > -            ASSERT(addr + size < MB1_PAGES);
>> > -            pvh_steal_low_ram(d, addr, size);
>> > -        }
>> > -
>> > +        rc = pvh_populate_memory_range(d, addr, size);
>> >          if ( rc )
>> >              return rc;
>> > +
>> > +        if ( addr < MB1_PAGES )
>> > +        {
>> > +            uint64_t end = min_t(uint64_t, MB(1),
>> > +                                 d->arch.e820[i].addr + 
>> > d->arch.e820[i].size);
>> 
>> I think min_t() and max_t() should only be used as a last resort, due
>> to their (hidden) casts. min(MB(1ULL), ...) ought to be fine here, I
>> would think.
> 
> MB(1ULL) doesn't work because the macro already appends ULL:
> 
> #define MB(_mb)     (_AC(_mb, ULL) << 20)

Oh, right - that's the unfortunate inconsistent mapping of uint64_t to
"unsigned long long" on 32-bit but "unsigned long" on 64-bit.

> So I either have to cast d->arch.e820[i].addr + d->arch.e820[i].size
> to unsigned long long, or use (uint64_t)MB(1).

In that case I'll rather withdraw my change request.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel