Re: [PATCH 1/3] via-ide: Fix legacy mode emulation

[BALATON Zoltan]

On Mon, 9 Oct 2023, Bernhard Beschow wrote:
> Am 8. Oktober 2023 11:08:58 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
> > On Sun, 8 Oct 2023, Mark Cave-Ayland wrote:
> > > On 05/10/2023 23:13, BALATON Zoltan wrote:
> > >
> > > > The initial value for BARs were set in reset method for emulating
> > > > legacy mode at start but this does not work because PCI code resets
> > > > BARs after calling device reset method.
> > >
> > > This is certainly something I've noticed when testing previous versions of the 
> > > VIA patches. Perhaps it's worth a separate thread to the PCI devs?
> >
> > I think I brought up this back then but was told current PCI code won't change 
> > and since that could break everything else that makes sense so this is 
> > something that we should take as given and accomodate that.
>
> Why not play safe like:
> 1. add a class property such as `reset_bar_addrs[PCI_NUM_REGIONS]`
> 2. set all elements to zero in `pci_device_class_init()`
> 3. respect `reset_bar_addrs` in `pci_reset_regions()`
> 4. assign the proper reset addresses of TYPE_VIA_IDE in `via_ide_class_init()`
>
> That would pretty obviously preserve the behavior of existing device 
> models while allowing TYPE_VIA_IDE to be reset properly. It would also 
> perform the main part of the workaround in the code that exhibits the 
> limitation, so the code could potentially be simplified at some point 
> without impacting all PCI device models.

That's a lot of complication for setting some values that will be 
overwritten first thing after reset. Either the guest sets native mode and 
writes the BARs itself so the reset values don't matter or it sets legacy 
mode when the workaround has to set legacy ports (that strangely don't 
match the reset values of the BARs) so sticking to those reset values 
makes no sense to me as it does not help to run any guest just makes the 
code more complex and harder to understand for no reason. So the patch 
with clarified comment as Mark asked should do for now. I'll send that in 
v2 later.

Regards,
BALATON Zoltan