<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">2017-07-23 18:57 GMT+03:00 Marcel Apfelbaum <span dir="ltr"><<a href="mailto:marcel@redhat.com" target="_blank">marcel@redhat.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-">On 23/07/2017 1:15, Aleksandr Bezzubikov wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
On PCI init PCI bridges may need some<br>
extra info about bus number to reserve, IO, memory and<br>
prefetchable memory limits. QEMU can provide this<br>
with special vendor-specific PCI capability.<br>
<br>
Sizes of limits match ones from<br>
PCI Type 1 Configuration Space Header,<br>
number of buses to reserve occupies only 1 byte<br>
since it is the size of Subordinate Bus Number register.<br>
<br>
</blockquote>
<br></span>
Hi Alexandr,<span class="gmail-"><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Signed-off-by: Aleksandr Bezzubikov <<a href="mailto:zuban32s@gmail.com" target="_blank">zuban32s@gmail.com</a>><br>
---<br>
hw/pci/pci_bridge.c | 27 +++++++++++++++++++++++++++<br>
include/hw/pci/pci_bridge.h | 18 ++++++++++++++++++<br>
2 files changed, 45 insertions(+)<br>
<br>
diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c<br>
index 720119b..8ec6c2c 100644<br>
--- a/hw/pci/pci_bridge.c<br>
+++ b/hw/pci/pci_bridge.c<br>
@@ -408,6 +408,33 @@ void pci_bridge_map_irq(PCIBridge *br, const char* bus_name,<br>
br->bus_name = bus_name;<br>
}<br>
+<br>
+int pci_bridge_help_cap_init(PCIDe<wbr>vice *dev, int cap_offset,<br>
</blockquote>
<br></span>
Can you please rename to something like<br>
'pci_bridge_qemu_cap_init' to be more specific?<span class="gmail-"><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ uint8_t bus_reserve, uint32_t io_limit,<br>
+ uint16_t mem_limit, uint64_t pref_limit,<br>
</blockquote>
<br>
<br></span>
I am not sure regarding "limit" suffix, this<br>
is a reservation, not a limitation.</blockquote><div><br></div><div>I'd like this fields names to match actual registers which</div><div>are going to get the values. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="gmail-h5"><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ Error **errp)<br>
+{<br>
+ size_t cap_len = sizeof(PCIBridgeQemuCap);<br>
+ PCIBridgeQemuCap cap;<br>
+<br>
+ cap.len = cap_len;<br>
+ cap.bus_res = bus_reserve;<br>
+ cap.io_lim = io_limit & 0xFF;<br>
+ cap.io_lim_upper = io_limit >> 8 & 0xFFFF;<br>
+ cap.mem_lim = mem_limit;<br>
+ cap.pref_lim = pref_limit & 0xFFFF;<br>
+ cap.pref_lim_upper = pref_limit >> 16 & 0xFFFFFFFF;<br>
+<br>
+ int offset = pci_add_capability(dev, PCI_CAP_ID_VNDR,<br>
+ cap_offset, cap_len, errp);<br>
+ if (offset < 0) {<br>
+ return offset;<br>
+ }<br>
+<br>
+ memcpy(dev->config + offset + 2, (char *)&cap + 2, cap_len - 2);<br>
+ return 0;<br>
+}<br>
+<br>
static const TypeInfo pci_bridge_type_info = {<br>
.name = TYPE_PCI_BRIDGE,<br>
.parent = TYPE_PCI_DEVICE,<br>
diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h<br>
index ff7cbaa..c9f642c 100644<br>
--- a/include/hw/pci/pci_bridge.h<br>
+++ b/include/hw/pci/pci_bridge.h<br>
@@ -67,4 +67,22 @@ void pci_bridge_map_irq(PCIBridge *br, const char* bus_name,<br>
#define PCI_BRIDGE_CTL_DISCARD_STATUS 0x400 /* Discard timer status */<br>
#define PCI_BRIDGE_CTL_DISCARD_SERR 0x800 /* Discard timer SERR# enable */<br>
+typedef struct PCIBridgeQemuCap {<br>
+ uint8_t id; /* Standard PCI capability header field */<br>
+ uint8_t next; /* Standard PCI capability header field */<br>
+ uint8_t len; /* Standard PCI vendor-specific capability header field */<br>
+ uint8_t bus_res;<br>
+ uint32_t pref_lim_upper;<br>
+ uint16_t pref_lim;<br>
+ uint16_t mem_lim;<br>
</blockquote>
<br></div></div>
This 32bit IOMEM, right?<span class="gmail-"><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ uint16_t io_lim_upper;<br>
+ uint8_t io_lim;<br>
</blockquote>
<br></span>
Why do we need io_lim and io_lim_upper?<br></blockquote><div><br></div><div><div>The idea was to be able to directly move the capability fields values</div><div>to the registers when actually using it (in firmware) code.</div><div>Secondly, it saves a little space by avoding usage 32-bit types</div></div><div>when 24-bit ones are desired. And the same thing with prefetchable (48->64).</div><div>But if it's more convenient no to split this value, I can do that.</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Thanks,<br>
Marcel<div class="gmail-HOEnZb"><div class="gmail-h5"><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ uint8_t padding;<br>
+} PCIBridgeQemuCap;<br>
+<br>
+int pci_bridge_help_cap_init(PCIDe<wbr>vice *dev, int cap_offset,<br>
+ uint8_t bus_reserve, uint32_t io_limit,<br>
+ uint16_t mem_limit, uint64_t pref_limit,<br>
+ Error **errp);<br>
+<br>
#endif /* QEMU_PCI_BRIDGE_H */<br>
<br>
</blockquote>
<br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div dir="ltr"><div><div dir="ltr">Alexander Bezzubikov</div></div></div></div>
</div></div>