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

Re: [PATCH v3 2/2] xen/pci: replace call to is_memory_hole to pci_check_bar



Hi Jan,

On 07/09/2022 10:07, Jan Beulich wrote:
On 07.09.2022 10:58, Julien Grall wrote:
Hi Jan & Stefano,

On 06/09/2022 09:53, Jan Beulich wrote:
On 03.09.2022 02:24, Stefano Stabellini wrote:
On Thu, 1 Sep 2022, Rahul Singh wrote:
@@ -363,6 +373,39 @@ int __init pci_host_bridge_mappings(struct domain *d)
       return 0;
   }
+static int is_bar_valid(const struct dt_device_node *dev,
+                        uint64_t addr, uint64_t len, void *data)
+{
+    struct pdev_bar *bar_data = data;
+    unsigned long s = mfn_x(bar_data->start);
+    unsigned long e = mfn_x(bar_data->end);
+
+    if ( (s <= e) && (s >= PFN_DOWN(addr)) && (e <= PFN_UP(addr + len - 1)) )
+        bar_data->is_valid =  true;


This patch looks good and you addressed all Jan's comment well. Before I
ack it, one question.

I know that you made this change to address Jan's comment but using
PFN_DOWN for the (s >= PFN_DOWN(addr)) check and PFN_UP for the (e <=
PFN_UP(addr + len - 1)) check means that we are relaxing the
requirements, aren't we?

I know that this discussion is a bit pointless because addr and len should
always be page aligned, and if they weren't it would be a mistake.

Hmmm.... Is that requirement written down somewhere?

What do you mean here? Isn't it quite obvious that every byte in the
address space may only be used for a single purpose? I.e. if a byte
is covered by a BAR, it cannot also be covered by a RAM region or
yet something else (e.g. MMIO beyond BARs of PCI devices). What
happens if BAR and RAM indeed overlap depends on fabric and chipset,
but it'll either result in chaos if two parties respond to a single
request on the bus, or it'll be (hopefully) deterministic (for any
individual system) which of the two takes "precedence".

I am well aware about that and I am not sure how you implied this is what I was referring to from what I wrote (in particular if you read the next sentence).

Stefano wrote that it would be a mistake if the address/length is not page-aligned. However, I am not aware from such requirement written down. It seems to be more an expected common sense that was IIRC not always respected on HW supporting multiple page-granularity.

Cheers,

--
Julien Grall



 


Rackspace

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