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

Re: [Xen-devel] [PATCH v2] CONFIG_XEN_PV breaks xen_create_contiguous_region on ARM


  • To: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien.grall@xxxxxxx>
  • From: Juergen Gross <jgross@xxxxxxxx>
  • Date: Tue, 30 Oct 2018 07:57:43 +0100
  • Autocrypt: addr=jgross@xxxxxxxx; prefer-encrypt=mutual; keydata= xsBNBFOMcBYBCACgGjqjoGvbEouQZw/ToiBg9W98AlM2QHV+iNHsEs7kxWhKMjrioyspZKOB ycWxw3ie3j9uvg9EOB3aN4xiTv4qbnGiTr3oJhkB1gsb6ToJQZ8uxGq2kaV2KL9650I1SJve dYm8Of8Zd621lSmoKOwlNClALZNew72NjJLEzTalU1OdT7/i1TXkH09XSSI8mEQ/ouNcMvIJ NwQpd369y9bfIhWUiVXEK7MlRgUG6MvIj6Y3Am/BBLUVbDa4+gmzDC9ezlZkTZG2t14zWPvx XP3FAp2pkW0xqG7/377qptDmrk42GlSKN4z76ELnLxussxc7I2hx18NUcbP8+uty4bMxABEB AAHNHkp1ZXJnZW4gR3Jvc3MgPGpncm9zc0BzdXNlLmRlPsLAeQQTAQIAIwUCU4xw6wIbAwcL CQgHAwIBBhUIAgkKCwQWAgMBAh4BAheAAAoJELDendYovxMvi4UH/Ri+OXlObzqMANruTd4N zmVBAZgx1VW6jLc8JZjQuJPSsd/a+bNr3BZeLV6lu4Pf1Yl2Log129EX1KWYiFFvPbIiq5M5 kOXTO8Eas4CaScCvAZ9jCMQCgK3pFqYgirwTgfwnPtxFxO/F3ZcS8jovza5khkSKL9JGq8Nk czDTruQ/oy0WUHdUr9uwEfiD9yPFOGqp4S6cISuzBMvaAiC5YGdUGXuPZKXLpnGSjkZswUzY d9BVSitRL5ldsQCg6GhDoEAeIhUC4SQnT9SOWkoDOSFRXZ+7+WIBGLiWMd+yKDdRG5RyP/8f 3tgGiB6cyuYfPDRGsELGjUaTUq3H2xZgIPfOwE0EU4xwFgEIAMsx+gDjgzAY4H1hPVXgoLK8 B93sTQFN9oC6tsb46VpxyLPfJ3T1A6Z6MVkLoCejKTJ3K9MUsBZhxIJ0hIyvzwI6aYJsnOew cCiCN7FeKJ/oA1RSUemPGUcIJwQuZlTOiY0OcQ5PFkV5YxMUX1F/aTYXROXgTmSaw0aC1Jpo w7Ss1mg4SIP/tR88/d1+HwkJDVW1RSxC1PWzGizwRv8eauImGdpNnseneO2BNWRXTJumAWDD pYxpGSsGHXuZXTPZqOOZpsHtInFyi5KRHSFyk2Xigzvh3b9WqhbgHHHE4PUVw0I5sIQt8hJq 5nH5dPqz4ITtCL9zjiJsExHuHKN3NZsAEQEAAcLAXwQYAQIACQUCU4xwFgIbDAAKCRCw3p3W KL8TL0P4B/9YWver5uD/y/m0KScK2f3Z3mXJhME23vGBbMNlfwbr+meDMrJZ950CuWWnQ+d+ Ahe0w1X7e3wuLVODzjcReQ/v7b4JD3wwHxe+88tgB9byc0NXzlPJWBaWV01yB2/uefVKryAf AHYEd0gCRhx7eESgNBe3+YqWAQawunMlycsqKa09dBDL1PFRosF708ic9346GLHRc6Vj5SRA UTHnQqLetIOXZm3a2eQ1gpQK9MmruO86Vo93p39bS1mqnLLspVrL4rhoyhsOyh0Hd28QCzpJ wKeHTd0MAWAirmewHXWPco8p1Wg+V+5xfZzuQY0f4tQxvOpXpt4gQ1817GQ5/Ed/wsDtBBgB CAAgFiEEhRJncuj2BJSl0Jf3sN6d1ii/Ey8FAlrd8NACGwIAgQkQsN6d1ii/Ey92IAQZFggA HRYhBFMtsHpB9jjzHji4HoBcYbtP2GO+BQJa3fDQAAoJEIBcYbtP2GO+TYsA/30H/0V6cr/W V+J/FCayg6uNtm3MJLo4rE+o4sdpjjsGAQCooqffpgA+luTT13YZNV62hAnCLKXH9n3+ZAgJ RtAyDWk1B/0SMDVs1wxufMkKC3Q/1D3BYIvBlrTVKdBYXPxngcRoqV2J77lscEvkLNUGsu/z W2pf7+P3mWWlrPMJdlbax00vevyBeqtqNKjHstHatgMZ2W0CFC4hJ3YEetuRBURYPiGzuJXU pAd7a7BdsqWC4o+GTm5tnGrCyD+4gfDSpkOT53S/GNO07YkPkm/8J4OBoFfgSaCnQ1izwgJQ jIpcG2fPCI2/hxf2oqXPYbKr1v4Z1wthmoyUgGN0LPTIm+B5vdY82wI5qe9uN6UOGyTH2B3p hRQUWqCwu2sqkI3LLbTdrnyDZaixT2T0f4tyF5Lfs+Ha8xVMhIyzNb1byDI5FKCb
  • Cc: Stefano Stabellini <stefanos@xxxxxxxxxx>, Nathan.Studer@xxxxxxxxxxxxxxx, Jeff.Kubascik@xxxxxxxxxxxxxxx, xen-devel@xxxxxxxxxxxxxxxxxxxx, vkuznets@xxxxxxxxxx, boris.ostrovsky@xxxxxxxxxx, Jarvis.Roach@xxxxxxxxxxxxxxx
  • Delivery-date: Tue, 30 Oct 2018 06:57:59 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Openpgp: preference=signencrypt

On 30/10/2018 00:28, Stefano Stabellini wrote:
> On Fri, 26 Oct 2018, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 10/26/18 7:04 PM, Stefano Stabellini wrote:
>>> From: Stefano Stabellini <stefanos@xxxxxxxxxx>
>>>
>>> xen_create_contiguous_region has now only an implementation if
>>> CONFIG_XEN_PV is defined. However, on ARM we never set CONFIG_XEN_PV but
>>> we do have an implementation of xen_create_contiguous_region which is
>>> required for swiotlb-xen to work correctly (although it just sets
>>> *dma_handle).
>>>
>>> Add a stub implementation of xen_remap_pfn: it should never be called on
>>> ARM but it is necessary for linking.
>>>
>>> This fixes a bug introduced by 16624390816c4c40df3d777b34665d3fd01e693d.
>>
>> Again, this should contain a tag "Fixes: sha1 ("commit title")" so it can be
>> picked for backporting automatically.
> 
> Yeah, I forgot about it, it should be definitely added.
> 
> 
>>> Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
>>> CC: Jeff.Kubascik@xxxxxxxxxxxxxxx
>>> CC: Jarvis.Roach@xxxxxxxxxxxxxxx
>>> CC: Nathan.Studer@xxxxxxxxxxxxxxx
>>> CC: vkuznets@xxxxxxxxxx
>>> CC: boris.ostrovsky@xxxxxxxxxx
>>> CC: jgross@xxxxxxxx
>>> ---
>>>   arch/arm/xen/mm.c     | 8 ++++++++
>>>   include/xen/xen-ops.h | 2 +-
>>>   2 files changed, 9 insertions(+), 1 deletion(-)
>>> ---
>>> Changes in v2:
>>> - improve commit message
>>> - add xen_remap_pfn stub implementation
>>> - use if defined()
>>> diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
>>> index 785d2a5..7230e22 100644
>>> --- a/arch/arm/xen/mm.c
>>> +++ b/arch/arm/xen/mm.c
>>> @@ -182,6 +182,14 @@ void xen_destroy_contiguous_region(phys_addr_t pstart,
>>> unsigned int order)
>>>   }
>>>   EXPORT_SYMBOL_GPL(xen_destroy_contiguous_region);
>>>   +int xen_remap_pfn(struct vm_area_struct *vma, unsigned long addr,
>>> +             xen_pfn_t *pfn, int nr, int *err_ptr, pgprot_t prot,
>>> +             unsigned int domid, bool no_translate, struct page **pages)
>>> +{
>>> +   return -EOPNOTSUPP;
>>> +}
>>> +EXPORT_SYMBOL_GPL(xen_remap_pfn);
>>
>> This does not match the "unimplemented" version in xen-ops.h.
>>
>> But I find this solution quite ugly. Why don't you split the #ifdef in two
>> below?
> 
> I am happy to follow the maintainers' opinion on this. I would keep
> those those definition together but I don't mind either way.
> 
> 
> Juergen, Boris,
> 
> What do you prefer?

I'd follow Julien's suggestion.


Juergen

_______________________________________________
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®.