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

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


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Alexandru Stefan ISAILA <aisaila@xxxxxxxxxxxxxxx>
  • Date: Tue, 3 Mar 2020 09:59:16 +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=LsoAG2jjO9Lppu8LqsaraO+SjbHYVaV59fsyTLx9hiw=; b=UOL0xvKto2N3tnOseRfdMY857w9mkH+XQz2yUgdUSi+u1QKT/uDyBOJ38gz9zyuzHoaPkyy2ZqWqlyD5J//72sELrVL4YqV39J5oGeNKCwL4K3rDXOhhRsXynvrK/lxbDbB0MjAnHP0sJYxMmboxUQi4cYPRNioP5Dv+URKsKs3FnH8Zbo1RYtyL2WR1olb8gns+RJ3MINJlHqccGs8O+14tai1JDgVphfY0h+hZXw9+DrqK1cbh48j+kr+uM1lNvT9tVERLPx7f0gbxA66M0OmLxSioXq55BDvHxrGR+84e41ZaZp51l1ONirc9uV0LFyOzC3A/oQdTMNAVhrjGTw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=kIxHNpoOA0aRamNz2Z7Tt3mxkIOVg6M6Yol5AnfW0PoRlfvtxCeFXeItdPZUCRUh/wXhh5VNn9mwhWIvYQOVg0IX5XEjoD20bskpPH5cCdt7JVTtmICZhHhwkHLF8vbaXB1TSAqE7I/PNUORCsJES7xUp1pzP1xZkcfZ7bIONFHncf/pqZaoBocwsZxa4tUZYLk66uon9ecKhYnKqCZZnm6noXgB5gQykQK7l9b6j94rqw4JYlJz0O9eUnH0J/u1FDfbuZuAxh0RoWxJ8iADsYfGDxH3tzrFA6gXt7MQGDtLe0JC/j4Hrtg2f/mZ16XeORl3VeVuMk9DQ7kNzifLIQ==
  • 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: Tue, 03 Mar 2020 09:59:34 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHV7Kc8f1s5+kS3/0OKIQZfqMdQDqg2o5AAgAAlAoD//9/PAIAAAxoA
  • Thread-topic: [PATCH V5] x86/altp2m: Hypercall to set altp2m view visibility


On 03.03.2020 11:48, Jan Beulich wrote:
> On 03.03.2020 10:43, Alexandru Stefan ISAILA wrote:
>>
>>
>> On 03.03.2020 11:30, Jan Beulich wrote:
>>> On 26.02.2020 14:18, Alexandru Stefan ISAILA wrote:
>>>> @@ -4840,6 +4841,19 @@ static int do_altp2m_op(
>>>>            break;
>>>>        }
>>>>    
>>>> +    case HVMOP_altp2m_set_visibility:
>>>> +    {
>>>> +        uint16_t idx = a.u.set_visibility.altp2m_idx;
>>>
>>> Why a fixed width type (and even one inefficient to deal with)?
>>> (One might even ask - why a local variable in the first place,
>>> when it's used ...
>>>
>>>> +        if ( a.u.set_visibility.pad )
>>>> +            rc = -EINVAL;
>>>> +        else if ( !altp2m_active(d) )
>>>> +            rc = -EOPNOTSUPP;
>>>> +        else
>>>> +            rc = p2m_set_altp2m_view_visibility(d, idx,
>>>> +                                                
>>>> a.u.set_visibility.visible);
>>>
>>> ... just once here.) The function takes "unsigned int" in any
>>> event.
>>
>> Sure, I can have this idx dropped and use the value in the structure.
>> I had that in place to have line size smaller and the code easy to read.
> 
> Dropping the variable is secondary - if you prefer you keep it, so
> be it. But if you keep it, its type should by in line with
> ./CODING_STYLE.
> 

Ah yes, you are right, I will change the type to unsigned int.


On the rc point you mentioned, I think it will be better to have a goto 
label there and have the cleanup on "out:"


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