On 27.07.22 13:15, Jan Beulich wrote:
Hello Jan
On 19.07.2022 19:42, Oleksandr Tyshchenko wrote:
@@ -527,6 +592,17 @@ static int cf_check init_bars(struct pci_dev *pdev)
if ( (val & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_IO )
{
bars[i].type = VPCI_BAR_IO;
+
+#ifndef CONFIG_X86
+ if ( !is_hwdom )
+ {
+ rc = vpci_add_register(pdev->vpci, empty_bar_read, NULL,
+ reg, 4, &bars[i]);
+ if ( rc )
+ goto fail;
+ }
+#endif
Since long term this can't be correct, it wants a TODO comment put next
to it.
Looking into the previous versions of this patch (up to V3) I failed to
find any changes in current version which hadn't been discussed (and
agreed in some form).
Could you please clarify what exactly can't be correct the long term,
for me to put the proper TODO here. Do you perhaps mean that TODO needs
to explain why we have to diverge?
@@ -553,34 +635,47 @@ static int cf_check init_bars(struct pci_dev *pdev)
bars[i].size = size;
bars[i].prefetchable = val & PCI_BASE_ADDRESS_MEM_PREFETCH;
- rc = vpci_add_register(pdev->vpci, vpci_hw_read32, bar_write, reg, 4,
- &bars[i]);
+ rc = vpci_add_register(pdev->vpci,
+ is_hwdom ? vpci_hw_read32 : guest_bar_read,
+ is_hwdom ? bar_write : guest_bar_write,
+ reg, 4, &bars[i]);
if ( rc )
- {
- pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd);
- return rc;
- }
+ goto fail;
}
- /* Check expansion ROM. */
- rc = pci_size_mem_bar(pdev->sbdf, rom_reg, &addr, &size, PCI_BAR_ROM);
- if ( rc > 0 && size )
+ /* Check expansion ROM: we do not handle ROM for guests. */
+ if ( is_hwdom )
This again can't be right long-term. Personally I'd prefer if the code
was (largely) left as is, with adjustments (with suitable TODO comments)
made on a much smaller scope only.
I can revive a comment that Oleksandr Andrushchenko provided for earlier
version by transforming into TODO:
ROM BAR is only handled for the hardware domain and for guest domains
there is a stub: at the moment PCI expansion ROM handling is supported
for x86 only and it might not be used by other architectures without
emulating x86. Other use-cases may include using that expansion ROM before
Xen boots, hence no emulation is needed in Xen itself. Or when a guest
wants to use the ROM code which seems to be rare.
But I'm not the maintainer of this
code - Roger may have a different view on this.
Well, let's wait for Roger's input here.
Jan
--
Regards,
Oleksandr Tyshchenko
