Re: [Xen-devel] [PATCH v3.1 12/15] xen/x86: populate PVHv2 Dom0 physical memory map

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.11.16 at 14:30, <roger.pau@xxxxxxxxxx> wrote:
> On Mon, Nov 28, 2016 at 04:41:22AM -0700, Jan Beulich wrote:
>> >>> On 28.11.16 at 12:26, <roger.pau@xxxxxxxxxx> wrote:
>> > On Fri, Nov 11, 2016 at 10:16:43AM -0700, Jan Beulich wrote:
>> >> >>> On 29.10.16 at 10:59, <roger.pau@xxxxxxxxxx> wrote:
>> >> > +            saved_current = current;
>> >> > +            set_current(v);
>> >> > +            rc = hvm_copy_to_guest_phys(d->arch.e820[i].addr,
>> >> > +                                        
>> >> > maddr_to_virt(d->arch.e820[i].addr),
>> >> > +                                        end - d->arch.e820[i].addr);
>> >> > +            set_current(saved_current);
>> >> 
>> >> If anything goes wrong here, how much confusion will result from
>> >> current being wrong? In particular, will this complicate debugging
>> >> of possible issues?
>> > 
>> > TBH, I'm not sure, current in this case is the idle domain, so trying to 
>> > execute 
>> > a hvm_copy_to_guest_phys with current being the idle domain, which from a 
>> > Xen 
>> > PoV is a PV vCPU, would probably result in some assert triggering in the 
>> > hvm_copy_to_guest_phys path (or at least I would expect so). Note that 
>> > this 
> 
>> > chunk of code is removed, since RAM regions below 1MB are now mapped as 
>> > p2m_ram_rw.
>> 
>> Even if this chunk of code no longer exists, iirc there  were a few
>> more instances of this current overriding, so unless they're all gone
>> now I still think this need considering (and ideally finding a better
>> solution, maybe along the lines of mapcache_override_current()).
> 
> This could be solved by making __hvm_copy take a struct domain param, but 
> this 
> is a very big change. I could also try to fix __hvm_copy so that we can set 
> an 
> override vcpu, much like mapcache_override_current (hvm_override_current?).

Well, as implied before: If there's provably no harm to debuggability,
then perhaps there's no real need for you to change your code. If,
however, there remains any doubt, then I specifically suggested
that override variant, knowing that handing a proper struct vcpu *
or struct domain * to the function would likely end up touching a lot
of code.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel