[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: Mon, 24 Feb 2020 09:06:21 +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=UfuV5NUr8gP+tVUml4jRCgn9kcSpKt9CpNFMGSFdDXg=; b=CkZjAxyRo5QmJ00Vv7S99abQJpzfRqg0qPIg+c3JJOT0MuOSjGcQaRIJDSvQ75WpsNxA2kUQ1ha34IQDOy6Z1n3WwXkk+DLp3eL86GwF6mkq+GAzFi/bPIYAwF52amUg71f/uTXtWWdCGcyhWsfNGYLIXN9kD5vy/FfJBL6ht4DD15nAYj7t5qlSgJgFQ9w/oXd6S3kc16BgxXCo9k1QuP4yY1xEdRH+Y5kMZmK1I4inFcg/yNjvV+cNJxtplRyTlXi6pYLyW82iADqrHCM66NPieP+ZUi6dszdzJd4dlDpjS5Ni4OSosgRJ/N8fJCh6cOhcBbHoEQgMh5rkFqhgZA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=AtKqQiU/qEu6NTeWPCJN7N/pk3QMG7aaHYU8xkrGWBEhCQ1NK7EzRwCFMUZ+W4+nN2L47ZKrJMXnisY/Bk3tKhH3EGb4PTQKzZn5+G0mkfy8oyL4Sl5/wJ8NAFZ+cIW28FG/xAU0png+b45hiWt/xWsf6DaFQ/3FbUsLJ+o5SXRVAUMuMAS9ZekSStAdnWs0EjgY/6kM58U/xFCW5ITLEewZjOLMRZjCQ+vClXNC9eSuAVitz2sJfhFtVlcFnMu/7Kg5RowAKeYyYdv523U9sWvizhAFkuyUnP8p2/ahD9DVLRyzjroviYUFtdzHmF+gg1tPOk8Vx9TsTYCDzbLiYw==
  • 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: Mon, 24 Feb 2020 09:06:35 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHV6JEygsFwXfT/ykedOiNCZrGg7Kgl2b8AgAQ4eIA=
  • Thread-topic: [PATCH V4] x86/altp2m: Hypercall to set altp2m view visibility


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.

>> @@ -2638,7 +2639,9 @@ int p2m_destroy_altp2m_by_id(struct domain *d, 
>> unsigned int idx)
>>           {
>>               p2m_reset_altp2m(d, idx, ALTP2M_DEACTIVATE);
>>               d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] =
>> -            mfn_x(INVALID_MFN);
>> +                mfn_x(INVALID_MFN);
>> +            d->arch.altp2m_working_eptp[array_index_nospec(idx, MAX_EPTP)] =
>> +                mfn_x(INVALID_MFN);
> 
> I appreciate you also adjusting the bogus indentation of the pre-
> existing line.
> 

Thanks,
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®.