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

Re: [PATCH v2] xen/pci: detect when BARs are not suitably positioned


  • To: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Thu, 27 Jan 2022 13:14:21 +0100
  • 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=eqpTnif0F9bY38cKDGOlfIum6QhbwVE2YJhicpSd+V4=; b=XrjRktZhf3gwuNeq/VW4xKN1LeIiasR+FXM0g9N9uUhemJJOW0MkPp55ambHT1XjYsF1puoRzBkYbsSWkikPcDUkNptIi1IL1gFkD/C0RhsVXZpO4qT1yQVozq68APXa8lY+/kdhXH6w+8nrOM/YVSIAuhHFfpGRQ2+TSQeQGvzgpRL5Hoh2XlM+88XUC2KM7R2JOtaFX6kDJGAxoIss/GbgYXvQc42DVkaB0JRwfIy6Kok4XTLp6zmZNrXXbUfiDqPtbqhAs0uiAhcyOc/04uDGyhIphNdE8b5AHhcZpcG7voHCQfwc2u4hFLUJhHOWGKei+TjJFh1IVUyCTJYzbw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=T1mekHaD3v7A8cbPk+egDGLTZy3qv1ddaQ2RuNgtQX+KxsubkSSxrFTndJtkX9Y4fw20fDlMo+kdnCjjmy/2mEt1GoRYGqAHZYabxpDgpR8mo6kFcch9Hw8T1eRnr9QVXFZSWnBA6VE1gyBIAQugTZMm1MM9xfxxDyeNVX5J7jrVi+HnamDFMob4TCDaqK2yi/HI8SA8GYJ4jrBAXI07yzL6EQsaZ53fiwlvMS4GStPoOemtTJCjOUudVxeFl5HLqCO9tXR4Jihr7jOAhExNwlIh7QfLI5P8Z5yseinIsP5F5x9KBAzZxmshqUJoal58CAgfZiU1kPhnwjAlbgObng==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Paul Durrant <paul@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Thu, 27 Jan 2022 12:14:32 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 27.01.2022 10:47, Roger Pau Monné wrote:
> On Thu, Jan 27, 2022 at 09:48:16AM +0100, Jan Beulich wrote:
>> On 27.01.2022 09:22, Roger Pau Monne wrote:
>>> --- a/xen/arch/arm/mm.c
>>> +++ b/xen/arch/arm/mm.c
>>> @@ -1625,6 +1625,17 @@ bool is_iomem_page(mfn_t mfn)
>>>      return !mfn_valid(mfn);
>>>  }
>>>  
>>> +bool is_iomem_range(paddr_t start, uint64_t size)
>>> +{
>>> +    unsigned int i;
>>> +
>>> +    for ( i = 0; i < PFN_UP(size); i++ )
>>> +        if ( !is_iomem_page(_mfn(PFN_DOWN(start) + i)) )
>>> +            return false;
>>> +
>>> +    return true;
>>> +}
>>
>> I'm afraid you can't very well call this non-RFC as long as this doesn't
>> scale. Or if you're building on this still being dead code on Arm, then
>> some kind of "fixme" annotation would be needed here (or the function be
>> omitted altogether, for Arm folks to sort out before they actually start
>> selecting the HAS_PCI Kconfig setting).
> 
> I was expecting the lack of 'RFC' tag to attract the attention of the
> maintainers for feedback.

Oh, I see.

>>> --- a/xen/arch/x86/mm.c
>>> +++ b/xen/arch/x86/mm.c
>>> @@ -783,6 +783,23 @@ bool is_iomem_page(mfn_t mfn)
>>>      return (page_get_owner(page) == dom_io);
>>>  }
>>>  
>>> +bool is_iomem_range(paddr_t start, uint64_t size)
>>> +{
>>> +    unsigned int i;
>>> +
>>> +    for ( i = 0; i < e820.nr_map; i++ )
>>> +    {
>>> +        const struct e820entry *entry = &e820.map[i];
>>> +
>>> +        /* Do not allow overlaps with any memory range. */
>>> +        if ( start < entry->addr + entry->size &&
>>> +             entry->addr < start + size )
>>> +            return false;
>>> +    }
>>> +
>>> +    return true;
>>> +}
>>
>> I should have realized (and pointed out) earlier that with the type
>> check dropped the function name no longer represents what the
>> function does. It now really is unsuitable for about anything other
>> that the checking of BARs.
> 
> is_devmem_range would be more suitable?

I'd make it even more tight and use e.g. is_pci_bar_range(). Or
maybe is_uncovered_range()?

> I will wrap this in an #ifdef HAS_PCI section now.

What would be the point of that, as long as X86 selects HAS_PCI
unconditionally? (Arguably one may want to turn off PCI support in
shim-only builds.)

Jan




 


Rackspace

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