Re: [Xen-devel] [PATCH v10 5/9] xen/x86: allow HVM guests to use hypercalls to bring up vCPUs

[Roger Pau Monnà <roger.pau@xxxxxxxxxx>]

El 10/12/15 a les 17.53, Jan Beulich ha escrit:
>>>> On 07.12.15 at 17:48, <roger.pau@xxxxxxxxxx> wrote:
>> Allow the usage of the VCPUOP_initialise, VCPUOP_up, VCPUOP_down,
>> VCPUOP_is_up, VCPUOP_get_physid and VCPUOP_send_nmi hypercalls from HVM
>> guests.
>>
>> This patch introduces a new structure (vcpu_hvm_context) that should be used
>> in conjuction with the VCPUOP_initialise hypercall in order to initialize
>> vCPUs for HVM guests.
>>
>> Signed-off-by: Roger Pau Monnà <roger.pau@xxxxxxxxxx>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> 
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> albeit I may fiddle with some of the messages in check_segment()
> upon committing, and pending clarification on ...
> 
>> +    if ( hvm_paging_enabled(v) && !paging_mode_hap(v->domain) )
>> +    {
>> +        /* Shadow-mode CR3 change. Check PDBR and update refcounts. */
>> +        struct page_info *page = get_page_from_gfn(v->domain,
>> +                                 v->arch.hvm_vcpu.guest_cr[3] >> PAGE_SHIFT,
>> +                                 NULL, P2M_ALLOC);
>> +        if ( !page )
>> +        {
>> +            gprintk(XENLOG_ERR, "Invalid CR3: %#lx\n",
>> +                    v->arch.hvm_vcpu.guest_cr[3]);
>> +            domain_crash(v->domain);
>> +            return -EINVAL;
>> +        }
> 
> ... why you crash the domain here when you don't on any on the
> earlier error paths.

I don't see any reason why we should crash the domain, I'm not sure
where the domain_crash call it's coming from, it's been here since the
first version of this patch.

If you want I can send a new version without the domain crash, or you
can amend it while committing. AFAICT removing the domain_crash call
doesn't have any side effects.

Roger.


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