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

Re: [PATCH v2 1/4] IOMMU/x86: switch to alternatives-call patching in further instances


  • To: Jan Beulich <jbeulich@xxxxxxxx>, "paul@xxxxxxx" <paul@xxxxxxx>
  • From: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
  • Date: Fri, 28 Jan 2022 10:54:10 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; 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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=TQckBNnbN2DjuIhUYz7tdGviOCUcVXp9z8pxSHh+1yE=; b=JlWPos8mApt0iovst+m8Xf2AL/KYcGV7L88c2mApSWFb0JETT7o3vJwRrOUFntHZ5PAUMBSFychM51ZQNuZYNNPEQanES41P7W5WDe1IqT/ucjv/Hf8c2lEi5rhnJKYLCH/q9kbR1GFaW6pB0QhAvKvVDQo7PN6NunQ5BtWD0QQLEJwDqYp/V5QLhRMkeP7cazIylFbHST1k9UfwiAii9ogE2PMYVX44lUHosOP2fwVlK0FHA52OB6bTz5DWKtFrtSVgLfGPjMV91kJMvexbLtZ9A01Bzmv8K9rkyM2uCHoUPK5tqvG0chp7SymFELGr7vP8JO1rbmWRbbkVKBMzvw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=K0WMKI2GvHL2fAkCA4Wc+gCEV0+vLoVweYx1tlS1QVRiJx2l4utwKoOT4bmbThIrwggaFgA4fi2MphSTRe17PiUsnWv7eHqe+uydSFWYUJtpnkd7J2PSBcUh3xzl0LotCgDuhnhmaOJjx5TBB2t003zEcRk34v8XgRrfsWGzL2+r+Sdv8CGUTBdE9cLz/4+qZolSYks6jdlOLThORwFoy4g9Wt6eSz5qV2z4TNMkMP0UtzRL4jizekwpHiW/jMROawZBYKGuIkzEGL7aATrsTs2/cElSL8YqcSJrbXHAG3c732nxmdgmED5AJxuXFGlDaXc2KLrq7tHjHmVZ6gerQQ==
  • Authentication-results: esa2.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Fri, 28 Jan 2022 10:54:26 +0000
  • Ironport-data: A9a23:rKooJKnoEjcqkKTAncOwe1ro5gxuIURdPkR7XQ2eYbSJt1+Wr1Gzt xIcUWCAaPrbMGX0LY1+aom/9U0P6pfTnN5mHgpur3gyRCMWpZLJC+rCIxarNUt+DCFioGGLT Sk6QoOdRCzhZiaE/n9BClVlxJVF/fngqoDUUYYoAQgsA180IMsdoUg7wbRh29Q42YPR7z6l4 rseneWOYDdJ5BYsWo4kw/rrRMRH5amaVJsw5zTSVNgT1LPsvyB94KE3fMldG0DQUIhMdtNWc s6YpF2PEsE1yD92Yj+tuu6TnkTn2dc+NyDW4pZdc/DKbhSvOkXee0v0XRYRQR4/ttmHozx+4 PlMkIyPaz8ABZHNwrgQSxpaHTBzMoQTrdcrIVDn2SCS50jPcn+qyPRyFkAme4Yf/46bA0kXq 6ZecmpUKEne2aTmm9pXScE17ignBODtMJkSpTdLyjbBAOx9aZvCX7/L9ZlT2zJYasVmQ6yPO JBENWoHgBLoTixTN3coL6kChuKlmUfCKwEGp2KMqv9ii4TU5FMoi+W8WDbPQfSaSMMQkkuGq 2bu+2XiHgpcJNGZ0SCC8H+nmqnIhyyTcJobCbmQ5vNsxlqJyQQ7ChcbSF+6qvmRkVOlVpRUL El8x8Y1hfFsrgrxFIC7BkDm5i7f1vIBZzZOO8E7zRGy6JL92DqYXDQJVj1DZs0ru+ZjEFTGy WS1t9/uADVutpicRnSc6qqYoFuOBMQFEYMRTXRaFFVYurEPtKl210uSFYg7TMZZm/WoQWmY/ tyckMQpa1z/Z+Yv3r7zw13IiinESnPhHl9svVW/so5IA2pEiG+Zi26AtACzARVodt/xory9U J4swZX2AAcmVsnlqcB1aL9RdIxFHt7cWNEmvXZhHoM66xOm8GO5cIZb7VlWfRk1aZxfIWS3P ReO6Gu9AaO/2lPwM8ebhKrqU6wXIVXIT4y5Bpg4kPIQCnSOSON31H43PhPBt4wcuEMtjbs+K f+mnTWEVh4n5VBc5GPuHY81iOZzrghnnD+7bc2lk3yPjOTPDFbIGeZtGAbfNYgRsfLbyDg5B v4CbaNmPT0FDr2nCsQWmKZORW03wY8TXMCv9JcPJ7fde2KL2ggJUpfs/F/oQKQ894x9nebU5 HCtHEhezVv0n3rcLguWLHtkbdvSsVxX9hrX5AQgYgSl3WYNe4Gq4PtNfpc7Z+B/pudi0eR1X 78OfMDZWqZDTTHO+jI8a5jhrdM9KET31FzWZyf1MiIie5NARhDS/oO2dAXY6yRTXDG8stEzo uP821qDE4YDXQlrEO3fdOmrkwGqpXEYle8rBxnIL9BfdV/C6o9vLyCt3PY7L9tVcUfIxyeA1 hbQChAd/LGfr4gw+djPpKaFs4b2TLcuQhsERzHWtO/kOzPb82yvxZ57fNyJJT2NBnnp/KiCZ PlOy62uOvMwg1sX4ZF3FKxmzPxi6oK39aNa1AltAF7Cc0+vVuF7OnCD0MRC6v9Ny7tetVfkU 06D4IAHa7CAOcejG18NPgs1KO+E0KhMyDXV6P00JmT85TN2o+XbARkDYUHUhXwPNqZxPaMk3 fwl6Zwf5AGIgxY3NsqL03JP/GOWI31cC6gqu/n22mMwZtbHHr2aXaHhNw==
  • Ironport-hdrordr: A9a23:iZUP36McGr8sDMBcTv2jsMiBIKoaSvp037BN7TEUdfU1SL38qy nApoV56faZslcssRIb9+xoWpPwJk80nKQdieN9AV7LZniBhILCFvAB0WKN+V3d8gTFh5dgPf gKScND4afLYmSSJ/yKmDVQaOxN/OW6
  • Ironport-sdr: hSuW8XG5L9RHa+42azXpKZcoC+5BLi4wI+8zLGLsnv/i5P8ZGIAM7vzmwHhh3KjBsywPiowSA4 D7EULPf1HFTQblN5Jjcc2phTzBAb6pdJ8nt/Zh8n7T8qnh36CIPUi+aRV3w9hqzMQbCWcCQPu7 STTKRpNb0CtZAH/QTu95+vKc6+LK7N7d1g1uBJZ07a2zdugocCFBI6uz57SWh4vHW+Cj5yOMFK XKlDzoKggtx2wOY13poPT7x2jSqEWAB6WoRBUj9hOyWBi9bq+Z4La2mmn3GSTnElwUs/R8xlci aG0AgNMJEZsymHPs0jlCjSni
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHYE4zoXz+e/Oq3aUuTOSXG/tztY6x4K5QAgAATQwCAAATRAA==
  • Thread-topic: [PATCH v2 1/4] IOMMU/x86: switch to alternatives-call patching in further instances

On 28/01/2022 10:36, Jan Beulich wrote:
> On 28.01.2022 10:28, Durrant, Paul wrote:
>> On 27/01/2022 14:47, Jan Beulich wrote:
>>> @@ -1457,24 +1462,24 @@ static int iommu_get_device_group(
>>>       if ( !is_iommu_enabled(d) || !ops->get_device_group_id )
>>>           return 0;
>>>   
>>> -    group_id = ops->get_device_group_id(seg, bus, devfn);
>>> +    group_id = iommu_call(ops, get_device_group_id, seg, bus, devfn);
>>>   
>>>       pcidevs_lock();
>>>       for_each_pdev( d, pdev )
>>>       {
>>> -        if ( (pdev->seg != seg) ||
>>> -             ((pdev->bus == bus) && (pdev->devfn == devfn)) )
>>> +        unsigned int b = pdev->bus;
>>> +        unsigned int df = pdev->devfn;
>>> +
>>> +        if ( (pdev->seg != seg) || ((b == bus) && (df == devfn)) )
>>>               continue;
>>>   
>>> -        if ( xsm_get_device_group(XSM_HOOK, (seg << 16) | (pdev->bus << 8) 
>>> | pdev->devfn) )
>>> +        if ( xsm_get_device_group(XSM_HOOK, (seg << 16) | (b << 8) | df) )
>>>               continue;
>>>   
>>> -        sdev_id = ops->get_device_group_id(seg, pdev->bus, pdev->devfn);
>>> +        sdev_id = iommu_call(ops, get_device_group_id, seg, b, df);
>>>           if ( (sdev_id == group_id) && (i < max_sdevs) )
>>>           {
>>> -            bdf = 0;
>>> -            bdf |= (pdev->bus & 0xff) << 16;
>>> -            bdf |= (pdev->devfn & 0xff) << 8;
>>> +            bdf = (b << 16) | (df << 8);
>> Don't we have a macro for this now? Probably best to start using it 
>> whilst modifying the code.
> We don't. And it would feel somewhat misleading to use PCI_BDF2(b, df) << 8
> here. The situation is even worse imo: Besides there not being a macro, I
> also cannot seem to find any documentation on this non-standard layout (BDF
> shifted left by 8). Yet then again I also can't spot any caller of
> xc_get_device_group() ...

I'm sure I already did the archaeology.

device groups were broken by a hypercall bounce buffering change 2 years
before the only caller was dropped with Xend.

This mess of a hypercall has demonstrably not been used in a decade.  I
firmly suggest dropping it, rather than wasting effort trying to unbreak
an interface which needs deleting anyway as the first step to doing
IOMMU groups.

~Andrew

 


Rackspace

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