[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH V4] x86/altp2m: Hypercall to set altp2m view visibility


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Alexandru Stefan ISAILA <aisaila@xxxxxxxxxxxxxxx>
  • Date: Wed, 26 Feb 2020 09:34:17 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=bitdefender.com; dmarc=pass action=none header.from=bitdefender.com; dkim=pass header.d=bitdefender.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=ZOqx6Y/baL9nMb2fYPMrhU1QIbVOFRxzPL//BKBIo+0=; b=IjEhWST/7xEB2+ekQalcTOdxqD1dAyDgesdCDPloYaKhUuyttQmIVBApgxFyRg5uVKir9YXLrsFiw61/IasCjyRYL/MhsXwjo0Tl8EpAuP9SqHmM89NiRz493XdYQACKQpRtpRa4FQQLYqAka0RxneDMQa/qWAKeMyEc+LQ+KLyNUGhKnF7G+MezDkiw/sUHa2g7Fj+rvK2bMnSVdNyTb5iLuinbFEBCQsCae2nf28kocKjEGN1ZspfjMT8N6sUjlmyr03TsS+RKiAo2HnuOVMde2/LxLOgggW4e4X9rZlFY+nJ7AnkmwQ3wJx5bvRdHCFXpe1Sf1vqVa6KKYhg0rA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=fB7FH4k/C0eWKae1cgHNEbEsRNzImfiI1aB52x0RZFS42rBKPH2BI2Ev9YZpkF/yDXrQxtRABlkHsE7GKE7Qz6mbQY8QAvoKZjOaLnG2mOHDp5HfYmJbX2Dws8j8rVFMyF3+F9h9PumcxV6Pa6N9m+o5ZNJAMCsksex0R97beL67qLjPyin0XlVaSb/aKrIzqC/BvxB89G53Tds13Z+O3y0LVxiOA4boJ5BWkT02hLo7Q4twvYYId8nfVr08unlw9JUMTMlg2OhkdE5mE5JnDZZoK8A7YEUhCHasb+Sl56daXzhMO2lwV0SHRhFM9myH4j6mA6pETW5SAwu1WVk4aw==
  • Authentication-results: spf=none (sender IP is ) smtp.mailfrom=aisaila@xxxxxxxxxxxxxxx;
  • Cc: Kevin Tian <kevin.tian@xxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Wei Liu <wl@xxxxxxx>, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Ian Jackson <ian.jackson@xxxxxxxxxxxxx>, Jun Nakajima <jun.nakajima@xxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Delivery-date: Wed, 26 Feb 2020 09:34:24 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHV6JEygsFwXfT/ykedOiNCZrGg7Kgl2b8AgARZ/4CAAwrxgA==
  • Thread-topic: [Xen-devel] [PATCH V4] x86/altp2m: Hypercall to set altp2m view visibility


On 24.02.2020 11:06, Alexandru Stefan ISAILA wrote:
> 
> 
> On 21.02.2020 18:39, Jan Beulich wrote:
>> On 21.02.2020 09:30, Alexandru Stefan ISAILA wrote:
>>> @@ -4835,6 +4836,26 @@ static int do_altp2m_op(
>>>            break;
>>>        }
>>>    
>>> +    case HVMOP_altp2m_set_visibility:
>>> +    {
>>> +        uint16_t idx = a.u.set_visibility.altp2m_idx;
>>> +
>>> +        if ( a.u.set_visibility.pad ||
>>> +             idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
>>
>> Why min() here? You only access MAX_EPTP-dimensioned arrays afaics. If
>> this is intentional, I think it deserves a comment.
> 
> I have min() here because the altp2m index should not be grater then
> MAX_ALTP2M. I know this is used as altp2m_eptp() index but the two are
> coupled.
> 
>>
>>> +             d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] ==
>>> +             mfn_x(INVALID_MFN) )
>>> +            rc = -EINVAL;
>>> +        else if ( !altp2m_active(d) )
>>> +            rc = -EOPNOTSUPP;
>>> +        else if ( a.u.set_visibility.visible )
>>> +            d->arch.altp2m_working_eptp[array_index_nospec(idx, MAX_EPTP)] 
>>> =
>>> +                d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)];
>>> +        else
>>> +            d->arch.altp2m_working_eptp[array_index_nospec(idx, MAX_EPTP)] 
>>> =
>>> +                mfn_x(INVALID_MFN);
>>> +        break;
>>
>> You don't seem to be holding any locks. At the very least this means
>> the in-range-index-is-valid check further up will have become stale
>> by the time you actually consume the slot.
>>
> 
> Good thing to point this out here. I will add altp2m_list_lock/unlock(d)
> to guard this check and operation.

I think it's better to move all the altp2m things in a new function, 
something like p2m_set_altp2m_view_visibility(), in p2m.c. This way 
there will be no additional include needed.

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.