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

Re: [PATCH] tools/libxc: use uint32_t for pirq in xc_domain_irq_permission


  • To: Igor Druzhinin <igor.druzhinin@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Thu, 8 Jul 2021 02:26:41 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.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=NCsyovaDGC5PS7i4ezcUxXHb33WhIjTf2IzjSzcvd1Q=; b=J67x/GkcJ0y8uwspKihAi/XhIgTrvBcUfJTMabme8YFQ/9j5ibqnWk1HleiTg4VCQiq61MUU7S3qyRGLmHPDOPOAUvqdJtY0a+UJGwrAJ3Sxzx2JuzmZjxLcjB4buq7tzDYGSfphHb8oYAenq6IlPAidq+s/QUFuCXsCdlMOFxPNKHhAuqjwSbP3SE4+5M13wOfvHOCmNNEfPlX4yyc3eogSPhWWvoVhYncTYKhtDc5DMKIFetV1FEWtc9MO2Ecrg5IEKyV/burtqB3axZn2kA+Zpjke63d7Dyx5KcPf+53O003d6vOpk6d/cBjyZeLjWyTo3as3XEEhm1IaIen1TQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=dlRZ9cP8lX1My1g7WRT39dqtrh1hzYY7n5P7GK4baWD/0oD6zzC03zC0/L3LmhNykxfjsgBDliCQUFFXkh1Aubr/Xzcv7oaC5ME3I83DOQnncwNjBzvHExhDyJknfoIKNwILWiYPz0ewchZL4/DPlJIB0SVdYD9a0O+MltH96P2gxjPydWUfxo4c1gml8b6d869UcZX+8E862YJ5rkFZOsEliqMBL3hb2XnCOWaACJmM7r0jfHxk+anFBudMBqbkkFaCjlHQilcXlk5Y0e1tlZfjY3xxvdSuBWxIUV4BN+pJkWZVKHWN6iSGMAiYOT0+gfUr+c99CwFXobMzRXlM4Q==
  • Authentication-results: esa5.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: <iwj@xxxxxxxxxxxxxx>, <wl@xxxxxxx>, <george.dunlap@xxxxxxxxxx>, <julien@xxxxxxx>, <sstabellini@xxxxxxxxxx>, <jgross@xxxxxxxx>, <christian.lindig@xxxxxxxxxx>, <dave@xxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Thu, 08 Jul 2021 01:27:11 +0000
  • Ironport-hdrordr: A9a23:+Q1cb69XwMiqQra+5Axuk+E2db1zdoMgy1knxilNoENuHPBwxv rAoB1E73PJYVYqOE3Jmbi7Sc29qADnhOBICO4qTMiftWjdyReVxeRZjLcKrAeQYBEWmtQts5 uINpIOdeEYbmIK/voSgjPIa+rIqePvmMvD6Ja8vhUdOD2CKZsQiDuRYjzrYnGeLzM2fKbReq Dsg/av6wDQA0j+Oa+Adwg4tqX41pL2vaOjRSRDKw8s6QGIgz/twLnmEyKA1hNbdz9U278t/U XMjgS8v8yYwrCG4y6Z81WWw4VdmdPnxNcGLMuQivINIjGprgqzfoxuV5CLoThwiuCy71QBls XKvn4bTopOwkKUWlvwjQrm2gHm3jprwWTl00WkjXzqptG8bC4mCuJa7LgpMCfx2g4FhpVRwa hL12WWu958FhXbhhnw4NDOSlVDile0m3w/iuQe5kYvErf2UIUh6bD3wXklV6vpREnBmcYa+a hVfYHhDc9tABanhyuzhBg3/DTENU5DbCtvQSA5y4aoOnZt7ShEJ+Zx/r1Xop46zuNLd3Bz3Z WODk1ZrsA7ciYoV9MKOA4ge7r7NoWfe2OBDIqtSW6XXJ3vfUi98KLK3A==
  • Ironport-sdr: Isg8KaNH0BqOMgKPYesgf240P3TpYEpXKRkDrKyn+gf94mdo5dUvib1y0SgLOhhX23j1H45CZm LvrRKmjd/hXWTHhb9iRmwcd79DAp36Kd2GtlauXImxkt/12W1Sft42NIrTlXGD8IbHsF1MNhm3 6RD1XKQdEZ3V+Auhx3WeG++IyHMaBYzJs8axa4FZrKDBZRFDWn10WgzFsJ8JV5eeWplkDI+5sk 6o2Zy1uIkaMbzsFVHEXKueXbJd3KXsiJFiPqLPFG2jV347bERszdJ8cqN7L98fTwX+xhvxX6S0 GuU=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 08/07/2021 02:14, Igor Druzhinin wrote:
> On 08/07/2021 02:11, Andrew Cooper wrote:
>> On 08/07/2021 02:08, Igor Druzhinin wrote:
>>> On 07/07/2021 10:19, Andrew Cooper wrote:
>>>> On 07/07/2021 08:46, Jan Beulich wrote:
>>>>>> --- a/tools/include/xenctrl.h
>>>>>> +++ b/tools/include/xenctrl.h
>>>>>> @@ -1385,7 +1385,7 @@ int xc_domain_ioport_permission(xc_interface
>>>>>> *xch,
>>>>>>      int xc_domain_irq_permission(xc_interface *xch,
>>>>>>                                 uint32_t domid,
>>>>>> -                             uint8_t pirq,
>>>>>> +                             uint32_t pirq,
>>>>>>                                 uint8_t allow_access);
>>>>> Take the opportunity and also change "allow_access" to bool? Or is
>>>>> use of bool prohibited in external interfaces?
>>>>
>>>> We've got bool's in the interface already.
>>>
>>> Where exactly? I couldn't find a single "bool".
>>
>> $ git grep -w bool -- :/tools/include/xen*.h
>> ../tools/include/xenctrl.h:1844:                          uint32_t
>> domid, bool restore,
>> ../tools/include/xenctrl.h:1846:                          unsigned int
>> nr_features, bool pae, bool itsc,
>> ../tools/include/xenctrl.h:1847:                          bool
>> nested_virt, const struct xc_xend_cpuid *xend);
>> ../tools/include/xenctrl.h:1954:int
>> xc_altp2m_get_domain_state(xc_interface *handle, uint32_t dom, bool
>> *state);
>> ../tools/include/xenctrl.h:1955:int
>> xc_altp2m_set_domain_state(xc_interface *handle, uint32_t dom, bool
>> state);
>>
>> and loads more.
>
> Are we ok to have different types in ABI interface and in libxc
> function prototype then?

Yes.  Again, we've got plenty of examples like this.

> Because I was referring to ABI structures.

The hypercall structs can't contain bool.  bool has implementation
defined width in C, just like enum, and there is no requirement for
sizeof(bool) to be 1.

The pre-existing uint8_t here is correct, although the hypercall handler
ideally wants a further adjustment to reject non-boolean values.  This
hypercall clearly predates our more careful review practices...

~Andrew

 


Rackspace

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