On Mon, Jan 31, 2022 at 09:53:29AM +0000, Oleksandr Andrushchenko wrote: > Hi, Roger! > > On 12.01.22 19:34, Roger Pau Monné wrote: > > A couple more comments I realized while walking the dog. > > > > On Thu, Nov 25, 2021 at 01:02:43PM +0200, Oleksandr Andrushchenko wrote: > >> From: Oleksandr Andrushchenko <[email protected]> > >> @@ -569,8 +625,10 @@ static int 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]); > > You need to initialize guest_reg to the physical host value also. > But why? There was a concern that exposing host's value to a guest > may be a security issue. And wouldn't it be possible for a guest to decide > that the firmware has setup the BAR and it doesn't need to care of it and > hence use a wrong value instead of setting it up by itself? I had an issue > with that if I'm not mistaken that guest's Linux didn't set the BAR properly > and used what was programmed
I think I've made that comment before realizing that all BARs must start with memory decoding disabled for guests, so that the guest firmware can position them. Using the host value as a starting point doesn't make sense because there's no relation between the guest and the host memory maps. You can drop this comment. Thanks, Roger.
