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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.12.15 at 18:18, <roger.pau@xxxxxxxxxx> wrote:
> 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.

I don't see a need for another version, unless other feedback you
might get would make that necessary.

Jan

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