| Message ID | BEDD3D6D-6ED9-4B23-AFAE-8ADB548D1ADE@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 29/12/15 17:59, Programmingkid wrote: > This patch solves the few problems that needed to be solved in order for a > Mac OS 10.4 guest to use the rtl8139 nic. > > The pci_dma_read() function would only return zero when a MemoryRegion object > was not enabled. Enabling the object makes the pci_dma_read() function work > correctly. > > The Linux rtl8139 driver needs base address register zero set to PIO. This > conflicts with Mac OS 10.4's driver needing base address register zero set to > MMIO. So a macro has been added that allows the user to decide which operating > system this network interface card will be compatible with. Note: Windows XP's > driver works regardless of the macro setting. > > Signed-off-by: John Arbuckle <programmingkidx@gmail.com> > > --- > hw/net/rtl8139.c | 15 ++++++++++++++- > include/hw/pci/pci.h | 1 + > 2 files changed, 15 insertions(+), 1 deletions(-) > > diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c > index 90ef72b..e17a471 100644 > --- a/hw/net/rtl8139.c > +++ b/hw/net/rtl8139.c > @@ -64,6 +64,14 @@ > /* debug RTL8139 card */ > //#define DEBUG_RTL8139 1 > > +/* > + * The driver that ships with Mac OS 10.4 has to have its base address register > + * 0 set to MMIO space. This directly conflicts with the Linux driver that > + * needs the PIO set to base address register 0. Mac OS 10.5 or higher does not > + * need this macro set. > + */ > +/* #define MAC_OS_10_4_DRIVER_COMPATIBILITY 1 */ > + > #define PCI_PERIOD 30 /* 30 ns period = 33.333333 Mhz frequency */ > > #define SET_MASKED(input, mask, curr) \ > @@ -3444,9 +3452,14 @@ static void pci_rtl8139_realize(PCIDevice *dev, Error **errp) > "rtl8139", 0x100); > memory_region_init_io(&s->bar_mem, OBJECT(s), &rtl8139_mmio_ops, s, > "rtl8139", 0x100); > + > +#ifdef MAC_OS_10_4_DRIVER_COMPATIBILITY > + pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->bar_mem); > + pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->bar_io); > +#else > pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->bar_io); > pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->bar_mem); > - > +#endif > qemu_macaddr_default_if_unset(&s->conf.macaddr); > > /* prepare eeprom */ > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h > index 379b6e1..8e95c75 100644 > --- a/include/hw/pci/pci.h > +++ b/include/hw/pci/pci.h > @@ -699,6 +699,7 @@ static inline int pci_dma_rw(PCIDevice *dev, dma_addr_t addr, > static inline int pci_dma_read(PCIDevice *dev, dma_addr_t addr, > void *buf, dma_addr_t len) > { > + memory_region_set_enabled(&dev->bus_master_enable_region, true); > return pci_dma_rw(dev, addr, buf, len, DMA_DIRECTION_TO_DEVICE); > } Can you provide more information about how you tested this patch? The ordering of the BARs is interesting - according to http://www.opensource.apple.com/source/AppleRTL8139Ethernet/AppleRTL8139Ethernet-141/RTL8139.cpp the driver tries to map the BARs like this: // Get the virtual address mapping of CSR registers located at // Base Address Range 0 (0x10). csrMap = pciNub->mapDeviceMemoryWithRegister( kIOPCIConfigBaseAddress1 ); if ( 0 == csrMap ) break; Now according to http://www.opensource.apple.com/source/IOPCIFamily/IOPCIFamily-151.5/IOKit/pci/IOPCIDevice.h kIOPCIConfigBaseAddress1 is actually BAR1 rather than BAR0, although in rtl8139.c both bar_mem and bar_io access the chip registers so I'm not sure why this would fail - can you try and trace the accesses through to determine exactly why the NIC doesn't work with the current BAR ordering? ATB, Mark.
On Dec 29, 2015, at 4:04 PM, Mark Cave-Ayland wrote: > On 29/12/15 17:59, Programmingkid wrote: > >> This patch solves the few problems that needed to be solved in order for a >> Mac OS 10.4 guest to use the rtl8139 nic. >> >> The pci_dma_read() function would only return zero when a MemoryRegion object >> was not enabled. Enabling the object makes the pci_dma_read() function work >> correctly. >> >> The Linux rtl8139 driver needs base address register zero set to PIO. This >> conflicts with Mac OS 10.4's driver needing base address register zero set to >> MMIO. So a macro has been added that allows the user to decide which operating >> system this network interface card will be compatible with. Note: Windows XP's >> driver works regardless of the macro setting. >> >> Signed-off-by: John Arbuckle <programmingkidx@gmail.com> >> >> --- >> hw/net/rtl8139.c | 15 ++++++++++++++- >> include/hw/pci/pci.h | 1 + >> 2 files changed, 15 insertions(+), 1 deletions(-) >> >> diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c >> index 90ef72b..e17a471 100644 >> --- a/hw/net/rtl8139.c >> +++ b/hw/net/rtl8139.c >> @@ -64,6 +64,14 @@ >> /* debug RTL8139 card */ >> //#define DEBUG_RTL8139 1 >> >> +/* >> + * The driver that ships with Mac OS 10.4 has to have its base address register >> + * 0 set to MMIO space. This directly conflicts with the Linux driver that >> + * needs the PIO set to base address register 0. Mac OS 10.5 or higher does not >> + * need this macro set. >> + */ >> +/* #define MAC_OS_10_4_DRIVER_COMPATIBILITY 1 */ >> + >> #define PCI_PERIOD 30 /* 30 ns period = 33.333333 Mhz frequency */ >> >> #define SET_MASKED(input, mask, curr) \ >> @@ -3444,9 +3452,14 @@ static void pci_rtl8139_realize(PCIDevice *dev, Error **errp) >> "rtl8139", 0x100); >> memory_region_init_io(&s->bar_mem, OBJECT(s), &rtl8139_mmio_ops, s, >> "rtl8139", 0x100); >> + >> +#ifdef MAC_OS_10_4_DRIVER_COMPATIBILITY >> + pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->bar_mem); >> + pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->bar_io); >> +#else >> pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->bar_io); >> pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->bar_mem); >> - >> +#endif >> qemu_macaddr_default_if_unset(&s->conf.macaddr); >> >> /* prepare eeprom */ >> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h >> index 379b6e1..8e95c75 100644 >> --- a/include/hw/pci/pci.h >> +++ b/include/hw/pci/pci.h >> @@ -699,6 +699,7 @@ static inline int pci_dma_rw(PCIDevice *dev, dma_addr_t addr, >> static inline int pci_dma_read(PCIDevice *dev, dma_addr_t addr, >> void *buf, dma_addr_t len) >> { >> + memory_region_set_enabled(&dev->bus_master_enable_region, true); >> return pci_dma_rw(dev, addr, buf, len, DMA_DIRECTION_TO_DEVICE); >> } > > Can you provide more information about how you tested this patch? The > ordering of the BARs is interesting - according to > http://www.opensource.apple.com/source/AppleRTL8139Ethernet/AppleRTL8139Ethernet-141/RTL8139.cpp > the driver tries to map the BARs like this: > > > // Get the virtual address mapping of CSR registers located at > // Base Address Range 0 (0x10). > > csrMap = pciNub->mapDeviceMemoryWithRegister( kIOPCIConfigBaseAddress1 ); > > if ( 0 == csrMap ) > break; > > > Now according to > http://www.opensource.apple.com/source/IOPCIFamily/IOPCIFamily-151.5/IOKit/pci/IOPCIDevice.h > kIOPCIConfigBaseAddress1 is actually BAR1 rather than BAR0, although in > rtl8139.c both bar_mem and bar_io access the chip registers so I'm not > sure why this would fail - can you try and trace the accesses through to > determine exactly why the NIC doesn't work with the current BAR ordering? I did add printf statements to the rtl8139_ioport_write() and rtl8139_ioport_read() functions, but they are never touched by the Mac OS X driver. All that happens is the driver's initAdapter() code would cause the driver to stop working. This is the code: if ( csrRead8( RTL_CM ) & R_CM_RST ) { // FIXME: need more robust recovery (retry?) IOLog("%s: chip reset timed out\n", getName()); return false; } The Darwin PPC iso you have might have the driver on it already. If you or anybody else needs help to test out the driver, just email me for help.
On Dec 29, 2015, at 7:05 PM, Programmingkid wrote: > > On Dec 29, 2015, at 4:04 PM, Mark Cave-Ayland wrote: > >> On 29/12/15 17:59, Programmingkid wrote: >> >>> This patch solves the few problems that needed to be solved in order for a >>> Mac OS 10.4 guest to use the rtl8139 nic. >>> >>> The pci_dma_read() function would only return zero when a MemoryRegion object >>> was not enabled. Enabling the object makes the pci_dma_read() function work >>> correctly. >>> >>> The Linux rtl8139 driver needs base address register zero set to PIO. This >>> conflicts with Mac OS 10.4's driver needing base address register zero set to >>> MMIO. So a macro has been added that allows the user to decide which operating >>> system this network interface card will be compatible with. Note: Windows XP's >>> driver works regardless of the macro setting. >>> >>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com> >>> >>> --- >>> hw/net/rtl8139.c | 15 ++++++++++++++- >>> include/hw/pci/pci.h | 1 + >>> 2 files changed, 15 insertions(+), 1 deletions(-) >>> >>> diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c >>> index 90ef72b..e17a471 100644 >>> --- a/hw/net/rtl8139.c >>> +++ b/hw/net/rtl8139.c >>> @@ -64,6 +64,14 @@ >>> /* debug RTL8139 card */ >>> //#define DEBUG_RTL8139 1 >>> >>> +/* >>> + * The driver that ships with Mac OS 10.4 has to have its base address register >>> + * 0 set to MMIO space. This directly conflicts with the Linux driver that >>> + * needs the PIO set to base address register 0. Mac OS 10.5 or higher does not >>> + * need this macro set. >>> + */ >>> +/* #define MAC_OS_10_4_DRIVER_COMPATIBILITY 1 */ >>> + >>> #define PCI_PERIOD 30 /* 30 ns period = 33.333333 Mhz frequency */ >>> >>> #define SET_MASKED(input, mask, curr) \ >>> @@ -3444,9 +3452,14 @@ static void pci_rtl8139_realize(PCIDevice *dev, Error **errp) >>> "rtl8139", 0x100); >>> memory_region_init_io(&s->bar_mem, OBJECT(s), &rtl8139_mmio_ops, s, >>> "rtl8139", 0x100); >>> + >>> +#ifdef MAC_OS_10_4_DRIVER_COMPATIBILITY >>> + pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->bar_mem); >>> + pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->bar_io); >>> +#else >>> pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->bar_io); >>> pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->bar_mem); >>> - >>> +#endif >>> qemu_macaddr_default_if_unset(&s->conf.macaddr); >>> >>> /* prepare eeprom */ >>> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h >>> index 379b6e1..8e95c75 100644 >>> --- a/include/hw/pci/pci.h >>> +++ b/include/hw/pci/pci.h >>> @@ -699,6 +699,7 @@ static inline int pci_dma_rw(PCIDevice *dev, dma_addr_t addr, >>> static inline int pci_dma_read(PCIDevice *dev, dma_addr_t addr, >>> void *buf, dma_addr_t len) >>> { >>> + memory_region_set_enabled(&dev->bus_master_enable_region, true); >>> return pci_dma_rw(dev, addr, buf, len, DMA_DIRECTION_TO_DEVICE); >>> } >> >> Can you provide more information about how you tested this patch? I would add printf statements to the functions that would be called in the RTL8139.c file when the driver is ran. The functions are these: rtl8139_io_writel(), rtl8139_io_readb(), rtl8139_io_readw(), rtl8139_io_readl(). The printf statements would print the register "addr" used and its value. The driver had its debug macro set when I compiled it, so I would see what functions were called. In the driver's outputPacket() function, I used a loop to print out all the raw packet data that was being sent to the nic. In the first row was the MAC address of the nic, so I know the data was not junk. I added a loop to the rtl8139.c file in the rtl8139_transmit_one() function that would also print the raw packet data. The data from the driver loop would be hexadecimal numbers, and usually all zeros in the rtl8139.c file's loop. That is how I knew there was a communication's error. After a lot of stepping thru code using gdb, I found out that a MemoryRegions's enabled member variable was set to false, causing a zero value being returned every time. When I set the value to true, that is when the magic started happening. For the first time, data between the driver and the emulated nic matched. It didn't take me too long to figure out where to put the code needed to make communications possible. I would build the Mac OS X driver inside of QEMU using Mac OS 10.4 as a guest. I used XCode to build the driver. To run the driver I used these commands: "sudo su" once to go into root. cp -R <path to xcode build directory>/AppleRTL8139.kext /tmp kextload /tmp/AppleRTL8139.kext Then review all the debug text I would see in the Console application. kextunload /tmp/AppleRTL8139.kext - unload the driver so I could test more changes without rebooting. I would sometimes go into the Network Pane inside of the System Preferences to see if the nic was working. After a lot of hard work, I was able to watch a YouTube video inside of QEMU with this patch applied.
On 30/12/15 00:35, Programmingkid wrote: > On Dec 29, 2015, at 7:05 PM, Programmingkid wrote: > >> >> On Dec 29, 2015, at 4:04 PM, Mark Cave-Ayland wrote: >> >>> On 29/12/15 17:59, Programmingkid wrote: >>> >>>> This patch solves the few problems that needed to be solved in order for a >>>> Mac OS 10.4 guest to use the rtl8139 nic. >>>> >>>> The pci_dma_read() function would only return zero when a MemoryRegion object >>>> was not enabled. Enabling the object makes the pci_dma_read() function work >>>> correctly. >>>> >>>> The Linux rtl8139 driver needs base address register zero set to PIO. This >>>> conflicts with Mac OS 10.4's driver needing base address register zero set to >>>> MMIO. So a macro has been added that allows the user to decide which operating >>>> system this network interface card will be compatible with. Note: Windows XP's >>>> driver works regardless of the macro setting. >>>> >>>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com> >>>> >>>> --- >>>> hw/net/rtl8139.c | 15 ++++++++++++++- >>>> include/hw/pci/pci.h | 1 + >>>> 2 files changed, 15 insertions(+), 1 deletions(-) >>>> >>>> diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c >>>> index 90ef72b..e17a471 100644 >>>> --- a/hw/net/rtl8139.c >>>> +++ b/hw/net/rtl8139.c >>>> @@ -64,6 +64,14 @@ >>>> /* debug RTL8139 card */ >>>> //#define DEBUG_RTL8139 1 >>>> >>>> +/* >>>> + * The driver that ships with Mac OS 10.4 has to have its base address register >>>> + * 0 set to MMIO space. This directly conflicts with the Linux driver that >>>> + * needs the PIO set to base address register 0. Mac OS 10.5 or higher does not >>>> + * need this macro set. >>>> + */ >>>> +/* #define MAC_OS_10_4_DRIVER_COMPATIBILITY 1 */ >>>> + >>>> #define PCI_PERIOD 30 /* 30 ns period = 33.333333 Mhz frequency */ >>>> >>>> #define SET_MASKED(input, mask, curr) \ >>>> @@ -3444,9 +3452,14 @@ static void pci_rtl8139_realize(PCIDevice *dev, Error **errp) >>>> "rtl8139", 0x100); >>>> memory_region_init_io(&s->bar_mem, OBJECT(s), &rtl8139_mmio_ops, s, >>>> "rtl8139", 0x100); >>>> + >>>> +#ifdef MAC_OS_10_4_DRIVER_COMPATIBILITY >>>> + pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->bar_mem); >>>> + pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->bar_io); >>>> +#else >>>> pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->bar_io); >>>> pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->bar_mem); >>>> - >>>> +#endif >>>> qemu_macaddr_default_if_unset(&s->conf.macaddr); >>>> >>>> /* prepare eeprom */ >>>> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h >>>> index 379b6e1..8e95c75 100644 >>>> --- a/include/hw/pci/pci.h >>>> +++ b/include/hw/pci/pci.h >>>> @@ -699,6 +699,7 @@ static inline int pci_dma_rw(PCIDevice *dev, dma_addr_t addr, >>>> static inline int pci_dma_read(PCIDevice *dev, dma_addr_t addr, >>>> void *buf, dma_addr_t len) >>>> { >>>> + memory_region_set_enabled(&dev->bus_master_enable_region, true); >>>> return pci_dma_rw(dev, addr, buf, len, DMA_DIRECTION_TO_DEVICE); >>>> } >>> >>> Can you provide more information about how you tested this patch? > > I would add printf statements to the functions that would be called in the RTL8139.c file when the driver is ran. The functions are these: rtl8139_io_writel(), rtl8139_io_readb(), rtl8139_io_readw(), rtl8139_io_readl(). The printf statements would print the register "addr" used and its value. > > The driver had its debug macro set when I compiled it, so I would see what functions were called. > > In the driver's outputPacket() function, I used a loop to print out all the raw packet data that was being sent to the nic. In the first row was the MAC address of the nic, so I know the data was not junk. I added a loop to the rtl8139.c file in the rtl8139_transmit_one() function that would also print the raw packet data. The data from the driver loop would be hexadecimal numbers, and usually all zeros in the rtl8139.c file's loop. That is how I knew there was a communication's error. > > After a lot of stepping thru code using gdb, I found out that a MemoryRegions's enabled member variable was set to false, causing a zero value being returned every time. When I set the value to true, that is when the magic started happening. For the first time, data between the driver and the emulated nic matched. It didn't take me too long to figure out where to put the code needed to make communications possible. > > I would build the Mac OS X driver inside of QEMU using Mac OS 10.4 as a guest. I used XCode to build the driver. To run the driver I used these commands: > > "sudo su" once to go into root. > cp -R <path to xcode build directory>/AppleRTL8139.kext /tmp > kextload /tmp/AppleRTL8139.kext > Then review all the debug text I would see in the Console application. > kextunload /tmp/AppleRTL8139.kext - unload the driver so I could test more changes without rebooting. > > I would sometimes go into the Network Pane inside of the System Preferences to see if the nic was working. After a lot of hard work, I was able to watch a YouTube video inside of QEMU with this patch applied. That's great detective work! As Zoltan mentioned in your other thread, what you're doing with the memory_region_set_enabled() call is the same as enabling bus mastering on the card, so can you try removing it from the patch and testing against an OpenBIOS with this patch applied instead: http://www.openfirmware.info/pipermail/openbios/2014-June/008448.html. It looks like we have to face up to the reality that Apple's OF implementation likes to enable bus mastering for some cards and emulate this behaviour in OpenBIOS, since because of this some drivers don't explicitly enable bus mastering and so fail under emulation. I'll go back and revisit Zoltan's patches to see if we can come up with something that works better for OpenBIOS upstream. In the meantime I still can't quite work out why you still need to swap the BARs around as AFAICT both the mmio and io BARs are mapped onto the chip registers - any chance you could add some debugging to QEMU in order to figure out what is going wrong without your patch? ATB, Mark.
On Dec 30, 2015, at 9:03 AM, Mark Cave-Ayland wrote: > On 30/12/15 00:35, Programmingkid wrote: > >> On Dec 29, 2015, at 7:05 PM, Programmingkid wrote: >> >>> >>> On Dec 29, 2015, at 4:04 PM, Mark Cave-Ayland wrote: >>> >>>> On 29/12/15 17:59, Programmingkid wrote: >>>> >>>>> This patch solves the few problems that needed to be solved in order for a >>>>> Mac OS 10.4 guest to use the rtl8139 nic. >>>>> >>>>> The pci_dma_read() function would only return zero when a MemoryRegion object >>>>> was not enabled. Enabling the object makes the pci_dma_read() function work >>>>> correctly. >>>>> >>>>> The Linux rtl8139 driver needs base address register zero set to PIO. This >>>>> conflicts with Mac OS 10.4's driver needing base address register zero set to >>>>> MMIO. So a macro has been added that allows the user to decide which operating >>>>> system this network interface card will be compatible with. Note: Windows XP's >>>>> driver works regardless of the macro setting. >>>>> >>>>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com> >>>>> >>>>> --- >>>>> hw/net/rtl8139.c | 15 ++++++++++++++- >>>>> include/hw/pci/pci.h | 1 + >>>>> 2 files changed, 15 insertions(+), 1 deletions(-) >>>>> >>>>> diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c >>>>> index 90ef72b..e17a471 100644 >>>>> --- a/hw/net/rtl8139.c >>>>> +++ b/hw/net/rtl8139.c >>>>> @@ -64,6 +64,14 @@ >>>>> /* debug RTL8139 card */ >>>>> //#define DEBUG_RTL8139 1 >>>>> >>>>> +/* >>>>> + * The driver that ships with Mac OS 10.4 has to have its base address register >>>>> + * 0 set to MMIO space. This directly conflicts with the Linux driver that >>>>> + * needs the PIO set to base address register 0. Mac OS 10.5 or higher does not >>>>> + * need this macro set. >>>>> + */ >>>>> +/* #define MAC_OS_10_4_DRIVER_COMPATIBILITY 1 */ >>>>> + >>>>> #define PCI_PERIOD 30 /* 30 ns period = 33.333333 Mhz frequency */ >>>>> >>>>> #define SET_MASKED(input, mask, curr) \ >>>>> @@ -3444,9 +3452,14 @@ static void pci_rtl8139_realize(PCIDevice *dev, Error **errp) >>>>> "rtl8139", 0x100); >>>>> memory_region_init_io(&s->bar_mem, OBJECT(s), &rtl8139_mmio_ops, s, >>>>> "rtl8139", 0x100); >>>>> + >>>>> +#ifdef MAC_OS_10_4_DRIVER_COMPATIBILITY >>>>> + pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->bar_mem); >>>>> + pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->bar_io); >>>>> +#else >>>>> pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->bar_io); >>>>> pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->bar_mem); >>>>> - >>>>> +#endif >>>>> qemu_macaddr_default_if_unset(&s->conf.macaddr); >>>>> >>>>> /* prepare eeprom */ >>>>> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h >>>>> index 379b6e1..8e95c75 100644 >>>>> --- a/include/hw/pci/pci.h >>>>> +++ b/include/hw/pci/pci.h >>>>> @@ -699,6 +699,7 @@ static inline int pci_dma_rw(PCIDevice *dev, dma_addr_t addr, >>>>> static inline int pci_dma_read(PCIDevice *dev, dma_addr_t addr, >>>>> void *buf, dma_addr_t len) >>>>> { >>>>> + memory_region_set_enabled(&dev->bus_master_enable_region, true); >>>>> return pci_dma_rw(dev, addr, buf, len, DMA_DIRECTION_TO_DEVICE); >>>>> } >>>> >>>> Can you provide more information about how you tested this patch? >> >> I would add printf statements to the functions that would be called in the RTL8139.c file when the driver is ran. The functions are these: rtl8139_io_writel(), rtl8139_io_readb(), rtl8139_io_readw(), rtl8139_io_readl(). The printf statements would print the register "addr" used and its value. >> >> The driver had its debug macro set when I compiled it, so I would see what functions were called. >> >> In the driver's outputPacket() function, I used a loop to print out all the raw packet data that was being sent to the nic. In the first row was the MAC address of the nic, so I know the data was not junk. I added a loop to the rtl8139.c file in the rtl8139_transmit_one() function that would also print the raw packet data. The data from the driver loop would be hexadecimal numbers, and usually all zeros in the rtl8139.c file's loop. That is how I knew there was a communication's error. >> >> After a lot of stepping thru code using gdb, I found out that a MemoryRegions's enabled member variable was set to false, causing a zero value being returned every time. When I set the value to true, that is when the magic started happening. For the first time, data between the driver and the emulated nic matched. It didn't take me too long to figure out where to put the code needed to make communications possible. >> >> I would build the Mac OS X driver inside of QEMU using Mac OS 10.4 as a guest. I used XCode to build the driver. To run the driver I used these commands: >> >> "sudo su" once to go into root. >> cp -R <path to xcode build directory>/AppleRTL8139.kext /tmp >> kextload /tmp/AppleRTL8139.kext >> Then review all the debug text I would see in the Console application. >> kextunload /tmp/AppleRTL8139.kext - unload the driver so I could test more changes without rebooting. >> >> I would sometimes go into the Network Pane inside of the System Preferences to see if the nic was working. After a lot of hard work, I was able to watch a YouTube video inside of QEMU with this patch applied. > > That's great detective work! As Zoltan mentioned in your other thread, > what you're doing with the memory_region_set_enabled() call is the same > as enabling bus mastering on the card, so can you try removing it from > the patch and testing against an OpenBIOS with this patch applied > instead: > http://www.openfirmware.info/pipermail/openbios/2014-June/008448.html. I did disable the change I made to the pci_dma_read() function and applied this patch to OpenBIOS. It makes Mac OS X stop booting. This is the message it is stuck at when Zoltan's patch is applied: mig_table_max_displ = 70 I even removed the rtl8139 from QEMU's options and Mac OS X still stopped booting at the same exact spot. > It looks like we have to face up to the reality that Apple's OF > implementation likes to enable bus mastering for some cards and emulate > this behaviour in OpenBIOS, since because of this some drivers don't > explicitly enable bus mastering and so fail under emulation. > > I'll go back and revisit Zoltan's patches to see if we can come up with > something that works better for OpenBIOS upstream. In the meantime I > still can't quite work out why you still need to swap the BARs around as > AFAICT both the mmio and io BARs are mapped onto the chip registers - > any chance you could add some debugging to QEMU in order to figure out > what is going wrong without your patch? I have done a lot of debugging. With only the pci_dma_read() change applied to QEMU, nothing the Mac OS X driver does ever accesses any of the functions from the rtl8139.c file (as far as I can tell). I will add that when this code "if ( csrRead8( RTL_CM ) & R_CM_RST )" is executed by the driver, the value is 16. It appeared to be pretty consistent. The value returned by just "csrRead8( RTL_CM )" is 24.
On 30/12/15 16:47, Programmingkid wrote: > I did disable the change I made to the pci_dma_read() function and applied this > patch to OpenBIOS. It makes Mac OS X stop booting. This is the message it is > stuck at when Zoltan's patch is applied: > mig_table_max_displ = 70 > > I even removed the rtl8139 from QEMU's options and Mac OS X still stopped booting > at the same exact spot. Okay. I've just posted a revised patchset to the OpenBIOS list which may (or may not) help, so please test with that aswell to see if it makes a difference. >> It looks like we have to face up to the reality that Apple's OF >> implementation likes to enable bus mastering for some cards and emulate >> this behaviour in OpenBIOS, since because of this some drivers don't >> explicitly enable bus mastering and so fail under emulation. >> >> I'll go back and revisit Zoltan's patches to see if we can come up with >> something that works better for OpenBIOS upstream. In the meantime I >> still can't quite work out why you still need to swap the BARs around as >> AFAICT both the mmio and io BARs are mapped onto the chip registers - >> any chance you could add some debugging to QEMU in order to figure out >> what is going wrong without your patch? > > I have done a lot of debugging. With only the pci_dma_read() change applied to QEMU, nothing the Mac OS X driver does ever accesses any of the functions from the rtl8139.c file (as far as I can tell). I will add that when this code "if ( csrRead8( RTL_CM ) & R_CM_RST )" is executed by the driver, the value is 16. It appeared to be pretty consistent. The value returned by just "csrRead8( RTL_CM )" is 24. Right, so then the question to ask is what is the difference in behaviours both before and after the BAR swap? It seems that you're getting a value back but presumably it must be a different value with your patch applied. ATB, Mark.
On Dec 30, 2015, at 11:58 AM, Mark Cave-Ayland wrote: > On 30/12/15 16:47, Programmingkid wrote: > >> I did disable the change I made to the pci_dma_read() function and applied this >> patch to OpenBIOS. It makes Mac OS X stop booting. This is the message it is >> stuck at when Zoltan's patch is applied: >> mig_table_max_displ = 70 >> >> I even removed the rtl8139 from QEMU's options and Mac OS X still stopped booting >> at the same exact spot. > > Okay. I've just posted a revised patchset to the OpenBIOS list which may > (or may not) help, so please test with that aswell to see if it makes a > difference. I have test your patch set. First I disabled my change to the pci_dma_read() function. Then I activated my change to the rtl8139.c file. The nic didn't work. The driver detected the nic, but DHCP did not set it up. Disabling my change to the rtl8139.c file causes a timeout message to be printed by the driver. When I was doing my research, I did a test where I changed the call to memory_region_set_enabled() in do_pci_register_device() to set everything to true instead of false. The MemoryRegion's enabled property was false by the time the RTL8139 tried using the pci_dma_read() function. I think something might be setting the enabled variable to false for some reason. > >>> It looks like we have to face up to the reality that Apple's OF >>> implementation likes to enable bus mastering for some cards and emulate >>> this behaviour in OpenBIOS, since because of this some drivers don't >>> explicitly enable bus mastering and so fail under emulation. >>> >>> I'll go back and revisit Zoltan's patches to see if we can come up with >>> something that works better for OpenBIOS upstream. In the meantime I >>> still can't quite work out why you still need to swap the BARs around as >>> AFAICT both the mmio and io BARs are mapped onto the chip registers - >>> any chance you could add some debugging to QEMU in order to figure out >>> what is going wrong without your patch? >> >> I have done a lot of debugging. With only the pci_dma_read() change applied to QEMU, nothing the Mac OS X driver does ever accesses any of the functions from the rtl8139.c file (as far as I can tell). I will add that when this code "if ( csrRead8( RTL_CM ) & R_CM_RST )" is executed by the driver, the value is 16. It appeared to be pretty consistent. The value returned by just "csrRead8( RTL_CM )" is 24. > > Right, so then the question to ask is what is the difference in > behaviours both before and after the BAR swap? It seems that you're > getting a value back but presumably it must be a different value with > your patch applied. The difference is I see activity in the RTL8139.c file when I made the swap. Without the change, none of the I/O functions in the RTL8139.c file are called. This is what I did. I added this code "printf("RTL8139: %s() called\n", __FUNCTION__);" to these functions in the rtl8139.c file: rtl8139_mmio_writeb() rtl8139_mmio_writew() rtl8139_mmio_writel() rtl8139_mmio_readb() rtl8139_mmio_readw() rtl8139_mmio_readl() rtl8139_ioport_write() rtl8139_ioport_read() If a register read and write from the driver doesn't access one of these functions, what else could it be accessing?
On 30/12/15 17:50, Programmingkid wrote: >> Okay. I've just posted a revised patchset to the OpenBIOS list which may >> (or may not) help, so please test with that aswell to see if it makes a >> difference. > > I have test your patch set. First I disabled my change to the pci_dma_read() function. Then I activated my change to the rtl8139.c file. The nic didn't work. The driver detected the nic, but DHCP did not set it up. Disabling my change to the rtl8139.c file causes a timeout message to be printed by the driver. > > When I was doing my research, I did a test where I changed the call to memory_region_set_enabled() in do_pci_register_device() to set everything to true instead of false. The MemoryRegion's enabled property was false by the time the RTL8139 tried using the pci_dma_read() function. I think something might be setting the enabled variable to false for some reason. I've managed to get something reasonably similar here with my Darwin image, and with the new OpenBIOS patchset then I get chip reset errors similar to you - but if I then just apply the BAR swap part of your patch then the error seemingly goes away. So this seems to confirm that the OpenBIOS patchset is doing the right thing and the call to memory_region_set_enabled() isn't required. >>>> It looks like we have to face up to the reality that Apple's OF >>>> implementation likes to enable bus mastering for some cards and emulate >>>> this behaviour in OpenBIOS, since because of this some drivers don't >>>> explicitly enable bus mastering and so fail under emulation. >>>> >>>> I'll go back and revisit Zoltan's patches to see if we can come up with >>>> something that works better for OpenBIOS upstream. In the meantime I >>>> still can't quite work out why you still need to swap the BARs around as >>>> AFAICT both the mmio and io BARs are mapped onto the chip registers - >>>> any chance you could add some debugging to QEMU in order to figure out >>>> what is going wrong without your patch? >>> >>> I have done a lot of debugging. With only the pci_dma_read() change applied to QEMU, nothing the Mac OS X driver does ever accesses any of the functions from the rtl8139.c file (as far as I can tell). I will add that when this code "if ( csrRead8( RTL_CM ) & R_CM_RST )" is executed by the driver, the value is 16. It appeared to be pretty consistent. The value returned by just "csrRead8( RTL_CM )" is 24. >> >> Right, so then the question to ask is what is the difference in >> behaviours both before and after the BAR swap? It seems that you're >> getting a value back but presumably it must be a different value with >> your patch applied. > > The difference is I see activity in the RTL8139.c file when I made the swap. Without the change, none of the I/O functions in the RTL8139.c file are called. > > This is what I did. I added this code "printf("RTL8139: %s() called\n", __FUNCTION__);" to these functions in the rtl8139.c file: > rtl8139_mmio_writeb() > rtl8139_mmio_writew() > rtl8139_mmio_writel() > rtl8139_mmio_readb() > rtl8139_mmio_readw() > rtl8139_mmio_readl() > rtl8139_ioport_write() > rtl8139_ioport_read() > > If a register read and write from the driver doesn't access one of these functions, what else could it be accessing? Yeah, I see that too. Given that you have the source to the Apple RTL8139 driver at hand, can you try with a patched OpenBIOS using the bus master patchset on the mailing list and also add some debugging to your copy of http://www.opensource.apple.com/source/AppleRTL8139Ethernet/AppleRTL8139Ethernet-141/RTL8139.cpp or similar like this to display the csrMap and csrBase variables: // Get the virtual address mapping of CSR registers located at // Base Address Range 0 (0x10). csrMap = pciNub->mapDeviceMemoryWithRegister( kIOPCIConfigBaseAddress1 ); DEBUG_LOG("csrMap: %x\n", csrMap); if ( 0 == csrMap ) break; csrBase = (volatile void*)csrMap->getVirtualAddress(); DEBUG_LOG("csrBase: %x\n", csrBase); At the very least these should give the addresses that the driver is trying to use in order to access the chip. If you can try this both before and after swapping the BARs over in QEMU that would be great. ATB, Mark.
On Dec 31, 2015, at 8:23 AM, Mark Cave-Ayland wrote: > On 30/12/15 17:50, Programmingkid wrote: > >>> Okay. I've just posted a revised patchset to the OpenBIOS list which may >>> (or may not) help, so please test with that aswell to see if it makes a >>> difference. >> >> I have test your patch set. First I disabled my change to the pci_dma_read() function. Then I activated my change to the rtl8139.c file. The nic didn't work. The driver detected the nic, but DHCP did not set it up. Disabling my change to the rtl8139.c file causes a timeout message to be printed by the driver. >> >> When I was doing my research, I did a test where I changed the call to memory_region_set_enabled() in do_pci_register_device() to set everything to true instead of false. The MemoryRegion's enabled property was false by the time the RTL8139 tried using the pci_dma_read() function. I think something might be setting the enabled variable to false for some reason. > > I've managed to get something reasonably similar here with my Darwin > image, and with the new OpenBIOS patchset then I get chip reset errors > similar to you - but if I then just apply the BAR swap part of your > patch then the error seemingly goes away. So this seems to confirm that > the OpenBIOS patchset is doing the right thing and the call to > memory_region_set_enabled() isn't required. The version of gdb that comes with Mac OS X is ancient. Since you are a Linux user, your version of gdb is probably a lot newer and better. Do you think you could add a watchpoint to the address of the enabled member variable to the MemoryRegion object of the RTL8139? If you did, we could find out when exactly the enabled variable is set to false. I think it would be easy for you to find this variable if you step thru the pci_dma_read() function with my patch applied. > >>>>> It looks like we have to face up to the reality that Apple's OF >>>>> implementation likes to enable bus mastering for some cards and emulate >>>>> this behaviour in OpenBIOS, since because of this some drivers don't >>>>> explicitly enable bus mastering and so fail under emulation. >>>>> >>>>> I'll go back and revisit Zoltan's patches to see if we can come up with >>>>> something that works better for OpenBIOS upstream. In the meantime I >>>>> still can't quite work out why you still need to swap the BARs around as >>>>> AFAICT both the mmio and io BARs are mapped onto the chip registers - >>>>> any chance you could add some debugging to QEMU in order to figure out >>>>> what is going wrong without your patch? >>>> >>>> I have done a lot of debugging. With only the pci_dma_read() change applied to QEMU, nothing the Mac OS X driver does ever accesses any of the functions from the rtl8139.c file (as far as I can tell). I will add that when this code "if ( csrRead8( RTL_CM ) & R_CM_RST )" is executed by the driver, the value is 16. It appeared to be pretty consistent. The value returned by just "csrRead8( RTL_CM )" is 24. >>> >>> Right, so then the question to ask is what is the difference in >>> behaviours both before and after the BAR swap? It seems that you're >>> getting a value back but presumably it must be a different value with >>> your patch applied. >> >> The difference is I see activity in the RTL8139.c file when I made the swap. Without the change, none of the I/O functions in the RTL8139.c file are called. >> >> This is what I did. I added this code "printf("RTL8139: %s() called\n", __FUNCTION__);" to these functions in the rtl8139.c file: >> rtl8139_mmio_writeb() >> rtl8139_mmio_writew() >> rtl8139_mmio_writel() >> rtl8139_mmio_readb() >> rtl8139_mmio_readw() >> rtl8139_mmio_readl() >> rtl8139_ioport_write() >> rtl8139_ioport_read() >> >> If a register read and write from the driver doesn't access one of these functions, what else could it be accessing? > > Yeah, I see that too. Given that you have the source to the Apple > RTL8139 driver at hand, can you try with a patched OpenBIOS using the > bus master patchset on the mailing list and also add some debugging to > your copy of > http://www.opensource.apple.com/source/AppleRTL8139Ethernet/AppleRTL8139Ethernet-141/RTL8139.cpp > or similar like this to display the csrMap and csrBase variables: > > > // Get the virtual address mapping of CSR registers located at > // Base Address Range 0 (0x10). > > csrMap = pciNub->mapDeviceMemoryWithRegister( kIOPCIConfigBaseAddress1 ); > > DEBUG_LOG("csrMap: %x\n", csrMap); > > if ( 0 == csrMap ) > break; > > csrBase = (volatile void*)csrMap->getVirtualAddress(); > > DEBUG_LOG("csrBase: %x\n", csrBase); > > > At the very least these should give the addresses that the driver is > trying to use in order to access the chip. If you can try this both > before and after swapping the BARs over in QEMU that would be great. Here are the results. Values with my patch fully applied (with pci_dma_read() change): csrMap: 0x270b3c0 csrBase: 0x20f8e000 I then kextunload'ed the kernel extension and loaded it again. Here are the values: csrMap: 0x270ffc0 csrBase: 0x20f8c000 Values with only the RTL8139.c changes applied (no pci_dma_read() change): csrMap: 0x1177740 csrBase: 0x1f5bf000 After unloading and loading the kernel extension: csrMap: 0x11776c0 csrBase: 0x1f5d6000
On 31/12/15 18:26, Programmingkid wrote: >> Yeah, I see that too. Given that you have the source to the Apple >> RTL8139 driver at hand, can you try with a patched OpenBIOS using the >> bus master patchset on the mailing list and also add some debugging to >> your copy of >> http://www.opensource.apple.com/source/AppleRTL8139Ethernet/AppleRTL8139Ethernet-141/RTL8139.cpp >> or similar like this to display the csrMap and csrBase variables: >> >> >> // Get the virtual address mapping of CSR registers located at >> // Base Address Range 0 (0x10). >> >> csrMap = pciNub->mapDeviceMemoryWithRegister( kIOPCIConfigBaseAddress1 ); >> >> DEBUG_LOG("csrMap: %x\n", csrMap); >> >> if ( 0 == csrMap ) >> break; >> >> csrBase = (volatile void*)csrMap->getVirtualAddress(); >> >> DEBUG_LOG("csrBase: %x\n", csrBase); >> >> >> At the very least these should give the addresses that the driver is >> trying to use in order to access the chip. If you can try this both >> before and after swapping the BARs over in QEMU that would be great. > > Here are the results. > > Values with my patch fully applied (with pci_dma_read() change): > csrMap: 0x270b3c0 > csrBase: 0x20f8e000 > > I then kextunload'ed the kernel extension and loaded it again. Here are the values: > csrMap: 0x270ffc0 > csrBase: 0x20f8c000 > > > Values with only the RTL8139.c changes applied (no pci_dma_read() change): > csrMap: 0x1177740 > csrBase: 0x1f5bf000 > > After unloading and loading the kernel extension: > csrMap: 0x11776c0 > csrBase: 0x1f5d6000 Well they are returning non-zero values, so that's good. After a bit more poking, something doesn't make sense - in that same file RTL8139::initPCIConfigSpace() claims to set the bus master bit, but if I add tracing to hw/pci/pci-host.c then I don't see any writes to the PCI command register outside of OpenBIOS? Any chance you can add debugging to the before and after values for the reg16 variable which this function claims to set? Also with similar tracing involved, apparently the writes to PCI I/O space in order to access the chip registers are actually going through pci_host_config_write(), i.e. configuration space rather than I/O space which is why they aren't accessing the chip registers. So more digging is required to figure out why this is happening. ATB, Mark.
On Dec 31, 2015, at 1:56 PM, Mark Cave-Ayland wrote: > On 31/12/15 18:26, Programmingkid wrote: > >>> Yeah, I see that too. Given that you have the source to the Apple >>> RTL8139 driver at hand, can you try with a patched OpenBIOS using the >>> bus master patchset on the mailing list and also add some debugging to >>> your copy of >>> http://www.opensource.apple.com/source/AppleRTL8139Ethernet/AppleRTL8139Ethernet-141/RTL8139.cpp >>> or similar like this to display the csrMap and csrBase variables: >>> >>> >>> // Get the virtual address mapping of CSR registers located at >>> // Base Address Range 0 (0x10). >>> >>> csrMap = pciNub->mapDeviceMemoryWithRegister( kIOPCIConfigBaseAddress1 ); >>> >>> DEBUG_LOG("csrMap: %x\n", csrMap); >>> >>> if ( 0 == csrMap ) >>> break; >>> >>> csrBase = (volatile void*)csrMap->getVirtualAddress(); >>> >>> DEBUG_LOG("csrBase: %x\n", csrBase); >>> >>> >>> At the very least these should give the addresses that the driver is >>> trying to use in order to access the chip. If you can try this both >>> before and after swapping the BARs over in QEMU that would be great. >> >> Here are the results. >> >> Values with my patch fully applied (with pci_dma_read() change): >> csrMap: 0x270b3c0 >> csrBase: 0x20f8e000 >> >> I then kextunload'ed the kernel extension and loaded it again. Here are the values: >> csrMap: 0x270ffc0 >> csrBase: 0x20f8c000 >> >> >> Values with only the RTL8139.c changes applied (no pci_dma_read() change): >> csrMap: 0x1177740 >> csrBase: 0x1f5bf000 >> >> After unloading and loading the kernel extension: >> csrMap: 0x11776c0 >> csrBase: 0x1f5d6000 > > Well they are returning non-zero values, so that's good. After a bit > more poking, something doesn't make sense - in that same file > RTL8139::initPCIConfigSpace() claims to set the bus master bit, but if I > add tracing to hw/pci/pci-host.c then I don't see any writes to the PCI > command register outside of OpenBIOS? Any chance you can add debugging > to the before and after values for the reg16 variable which this > function claims to set? You are right! Good job catching this! The value before the write is 0. The value after the write is still 0. The value it should be after the write is 21. > > Also with similar tracing involved, apparently the writes to PCI I/O > space in order to access the chip registers are actually going through > pci_host_config_write(), i.e. configuration space rather than I/O space > which is why they aren't accessing the chip registers. So more digging > is required to figure out why this is happening. Which target are you using: mac99 or g3beige. I use the g3beige target. Mac OS 10.4 is not suppose to be able to run on this machine, so I wonder if that has something to do with it. I did try to run Mac OS 10.4 in the mac99 target, but it just does not run very well there. It was unusable. The mouse didn't seem to work right. The console application didn't even load. I had to give up on that target. I do still plan on reviewing your patch set for fixing booting on the mac99 target. I just haven't found the time to do it yet.
On 29/12/2015 18:59, Programmingkid wrote: > @@ -699,6 +699,7 @@ static inline int pci_dma_rw(PCIDevice *dev, dma_addr_t addr, > static inline int pci_dma_read(PCIDevice *dev, dma_addr_t addr, > void *buf, dma_addr_t len) > { > + memory_region_set_enabled(&dev->bus_master_enable_region, true); > return pci_dma_rw(dev, addr, buf, len, DMA_DIRECTION_TO_DEVICE); > } In other words, the driver is buggy and presumably would have never worked on real hardware? Paolo
On Jan 1, 2016, at 3:32 PM, Paolo Bonzini wrote: > > > On 29/12/2015 18:59, Programmingkid wrote: >> @@ -699,6 +699,7 @@ static inline int pci_dma_rw(PCIDevice *dev, dma_addr_t addr, >> static inline int pci_dma_read(PCIDevice *dev, dma_addr_t addr, >> void *buf, dma_addr_t len) >> { >> + memory_region_set_enabled(&dev->bus_master_enable_region, true); >> return pci_dma_rw(dev, addr, buf, len, DMA_DIRECTION_TO_DEVICE); >> } > > In other words, the driver is buggy and presumably would have never > worked on real hardware? > > Paolo I can't say that for sure. I would need an actual RTL8139 nic with a PowerMac running Mac OS 10.4 to answer that question. Is there anyone on the list who knows for sure?
On 30/12/2015 01:35, Programmingkid wrote: > After a lot of stepping thru code using gdb, I found out that a > MemoryRegions's enabled member variable was set to false, causing a zero > value being returned every time. When I set the value to true, that is > when the magic started happening. For the first time, data between the > driver and the emulated nic matched. It didn't take me too long to > figure out where to put the code needed to make communications possible. > > I would build the Mac OS X driver inside of QEMU using Mac OS 10.4 > as a guest. I used XCode to build the driver. To run the driver I used > these commands: > > "sudo su" once to go into root. > cp -R <path to xcode build directory>/AppleRTL8139.kext /tmp > kextload /tmp/AppleRTL8139.kext > Then review all the debug text I would see in the Console application. > kextunload /tmp/AppleRTL8139.kext - unload the driver so I could test more changes without rebooting. > > I would sometimes go into the Network Pane inside of the System > Preferences to see if the nic was working. After a lot of hard work, > I was able to watch a YouTube video inside of QEMU with this patch > applied. Since you have the source, why don't you fix the driver instead? These changes to QEMU, unfortunately, are not correct. Reordering the BARs breaks Linux in all likelihood, and the bus master enable bit is there to avoid undesired memory corruptions on e.g. reset. (This does not cover the fact that #ifdefs are not the way to handle backwards-compatibility; 99% of the QEMU users do not build from source). I guess you _could_ add a property to ignore the bus master enable bit on a per-device basis (or per-class using -global). But still, fixing the driver seems much, much simpler. Both reordering the BARs, and setting the bus master bit in the command register after initialization is complete, should be easy to do. Paolo
On Jan 1, 2016, at 4:36 PM, Paolo Bonzini wrote: > > > On 30/12/2015 01:35, Programmingkid wrote: >> After a lot of stepping thru code using gdb, I found out that a >> MemoryRegions's enabled member variable was set to false, causing a zero >> value being returned every time. When I set the value to true, that is >> when the magic started happening. For the first time, data between the >> driver and the emulated nic matched. It didn't take me too long to >> figure out where to put the code needed to make communications possible. >> >> I would build the Mac OS X driver inside of QEMU using Mac OS 10.4 >> as a guest. I used XCode to build the driver. To run the driver I used >> these commands: >> >> "sudo su" once to go into root. >> cp -R <path to xcode build directory>/AppleRTL8139.kext /tmp >> kextload /tmp/AppleRTL8139.kext >> Then review all the debug text I would see in the Console application. >> kextunload /tmp/AppleRTL8139.kext - unload the driver so I could test more changes without rebooting. >> >> I would sometimes go into the Network Pane inside of the System >> Preferences to see if the nic was working. After a lot of hard work, >> I was able to watch a YouTube video inside of QEMU with this patch >> applied. > > Since you have the source, why don't you fix the driver instead? I don't offer a fixed driver because I honestly don't know that the driver is broken. It is also a lot more convenient for the user to just switch on a macro than have to install a custom driver (IMHO). > > These changes to QEMU, unfortunately, are not correct. Reordering the > BARs breaks Linux in all likelihood, and the bus master enable bit is > there to avoid undesired memory corruptions on e.g. reset. (This does > not cover the fact that #ifdefs are not the way to handle > backwards-compatibility; 99% of the QEMU users do not build from source). I didn't know that many users of QEMU do not build from source. I thought it was the other way around. I don't like this solution also. A one size fits all solution is what I was hoping for. > I guess you _could_ add a property to ignore the bus master enable bit > on a per-device basis (or per-class using -global). But still, fixing > the driver seems much, much simpler. Both reordering the BARs, and > setting the bus master bit in the command register after initialization > is complete, should be easy to do. Mark and I are working on an issue with the bus master bit in the command register. It appears to be set to false sometime after Mac OS X starts booting. I only found this out yesterday. Happy New Years to all
On 01/01/2016 21:42, Programmingkid wrote: > > In other words, the driver is buggy and presumably would have never > > worked on real hardware? > > I can't say that for sure. I would need an actual RTL8139 nic with a PowerMac running > Mac OS 10.4 to answer that question. Is there anyone on the list who knows for sure? I have a PowerBook and a RTL8139, but unfortunately the NIC is PCMCIA and the PowerBook doesn't have a port... If OpenBIOS can be fixed to work around any bugs in the driver that's already a good thing, but unfortunately the swapped BARs seem really fishy. :( Perhaps a property can be added for that, though (e.g. -global rtl8139.swap-bars-for-os-x-10-4=true). Paolo
On Jan 1, 2016, at 5:36 PM, Paolo Bonzini wrote: > > > On 01/01/2016 21:42, Programmingkid wrote: >>> In other words, the driver is buggy and presumably would have never >>> worked on real hardware? >> >> I can't say that for sure. I would need an actual RTL8139 nic with a PowerMac running >> Mac OS 10.4 to answer that question. Is there anyone on the list who knows for sure? > > I have a PowerBook and a RTL8139, but unfortunately the NIC is PCMCIA > and the PowerBook doesn't have a port... > > If OpenBIOS can be fixed to work around any bugs in the driver that's > already a good thing, but unfortunately the swapped BARs seem really > fishy. :( Perhaps a property can be added for that, though (e.g. > -global rtl8139.swap-bars-for-os-x-10-4=true). > > Paolo I really like the idea of having a runtime change of settings rather than a compile-time change. That would make QEMU much more dynamic.
On Dec 31, 2015, at 1:56 PM, Mark Cave-Ayland wrote: > On 31/12/15 18:26, Programmingkid wrote: > >>> Yeah, I see that too. Given that you have the source to the Apple >>> RTL8139 driver at hand, can you try with a patched OpenBIOS using the >>> bus master patchset on the mailing list and also add some debugging to >>> your copy of >>> http://www.opensource.apple.com/source/AppleRTL8139Ethernet/AppleRTL8139Ethernet-141/RTL8139.cpp >>> or similar like this to display the csrMap and csrBase variables: >>> >>> >>> // Get the virtual address mapping of CSR registers located at >>> // Base Address Range 0 (0x10). >>> >>> csrMap = pciNub->mapDeviceMemoryWithRegister( kIOPCIConfigBaseAddress1 ); >>> >>> DEBUG_LOG("csrMap: %x\n", csrMap); >>> >>> if ( 0 == csrMap ) >>> break; >>> >>> csrBase = (volatile void*)csrMap->getVirtualAddress(); >>> >>> DEBUG_LOG("csrBase: %x\n", csrBase); >>> >>> >>> At the very least these should give the addresses that the driver is >>> trying to use in order to access the chip. If you can try this both >>> before and after swapping the BARs over in QEMU that would be great. >> >> Here are the results. >> >> Values with my patch fully applied (with pci_dma_read() change): >> csrMap: 0x270b3c0 >> csrBase: 0x20f8e000 >> >> I then kextunload'ed the kernel extension and loaded it again. Here are the values: >> csrMap: 0x270ffc0 >> csrBase: 0x20f8c000 >> >> >> Values with only the RTL8139.c changes applied (no pci_dma_read() change): >> csrMap: 0x1177740 >> csrBase: 0x1f5bf000 >> >> After unloading and loading the kernel extension: >> csrMap: 0x11776c0 >> csrBase: 0x1f5d6000 > > Well they are returning non-zero values, so that's good. After a bit > more poking, something doesn't make sense - in that same file > RTL8139::initPCIConfigSpace() claims to set the bus master bit, but if I > add tracing to hw/pci/pci-host.c then I don't see any writes to the PCI > command register outside of OpenBIOS? Any chance you can add debugging > to the before and after values for the reg16 variable which this > function claims to set? > > Also with similar tracing involved, apparently the writes to PCI I/O > space in order to access the chip registers are actually going through > pci_host_config_write(), i.e. configuration space rather than I/O space > which is why they aren't accessing the chip registers. So more digging > is required to figure out why this is happening. > I added code to the initPCIConfigSpace() function that prints every register from 0 to 0x3e. All of the registers returned zero. My patch was fully applied when the driver printed all these values. I even had all the registers set to 0x99 then printed all the registers again. The registers all returned zero again. The driver was able to obtain the correct device, function, and bus numbers. PCI configuration space is not being accessed correctly by this driver.
On 02/01/16 04:00, Programmingkid wrote: >> Well they are returning non-zero values, so that's good. After a bit >> more poking, something doesn't make sense - in that same file >> RTL8139::initPCIConfigSpace() claims to set the bus master bit, but if I >> add tracing to hw/pci/pci-host.c then I don't see any writes to the PCI >> command register outside of OpenBIOS? Any chance you can add debugging >> to the before and after values for the reg16 variable which this >> function claims to set? >> >> Also with similar tracing involved, apparently the writes to PCI I/O >> space in order to access the chip registers are actually going through >> pci_host_config_write(), i.e. configuration space rather than I/O space >> which is why they aren't accessing the chip registers. So more digging >> is required to figure out why this is happening. >> > > I added code to the initPCIConfigSpace() function that prints every register from 0 to 0x3e. All of the registers returned zero. My patch was fully applied when the driver printed all these values. I even had all the registers set to 0x99 then printed all the registers again. The registers all returned zero again. The driver was able to obtain the correct device, function, and bus numbers. PCI configuration space is not being accessed correctly by this driver. After some head scratching chasing this through for several hours yesterday, I've worked out what is happening and have a patchset for OpenBIOS which should fix the issue (will post later along with some other PCI fixes for testing). This is actually a bug in Darwin in that it doesn't parse the address space part of the "ranges" property for the /pci node. OpenBIOS adds all 3 spaces to the "ranges" - configuration, I/O and memory compared to real Macs which only add I/O and memory. Which is generally fine because each entry contains a 2-bit indicator so that the OS knows which entry represents each address space. Except that Darwin evidently doesn't bother parsing the address space from each "range" entry and blindly assumes that entry 0 is I/O space and entry 1 is memory space as would be found in a real Mac device tree. The result of this is that currently I/O accesses go to entry 0 (config space) and memory accesses go to entry 1 (I/O space) which is why the RTL8139 happens to work with your patch to force bus mastering. AFAICT with basic testing here, forcing the /pci node to mimic that of a real Mac by omitting the configuration space entry seems to fix all PCI accesses (or at least they get routed to the correct address spaces), I see an attempt to set the bus master bit in the command register, and I don't get a reset timeout on startup for the RTL8139 anymore. ATB, Mark.
On Jan 2, 2016, at 4:39 AM, Mark Cave-Ayland wrote: > On 02/01/16 04:00, Programmingkid wrote: > >>> Well they are returning non-zero values, so that's good. After a bit >>> more poking, something doesn't make sense - in that same file >>> RTL8139::initPCIConfigSpace() claims to set the bus master bit, but if I >>> add tracing to hw/pci/pci-host.c then I don't see any writes to the PCI >>> command register outside of OpenBIOS? Any chance you can add debugging >>> to the before and after values for the reg16 variable which this >>> function claims to set? >>> >>> Also with similar tracing involved, apparently the writes to PCI I/O >>> space in order to access the chip registers are actually going through >>> pci_host_config_write(), i.e. configuration space rather than I/O space >>> which is why they aren't accessing the chip registers. So more digging >>> is required to figure out why this is happening. >>> >> >> I added code to the initPCIConfigSpace() function that prints every register from 0 to 0x3e. All of the registers returned zero. My patch was fully applied when the driver printed all these values. I even had all the registers set to 0x99 then printed all the registers again. The registers all returned zero again. The driver was able to obtain the correct device, function, and bus numbers. PCI configuration space is not being accessed correctly by this driver. > > After some head scratching chasing this through for several hours > yesterday, I've worked out what is happening and have a patchset for > OpenBIOS which should fix the issue (will post later along with some > other PCI fixes for testing). > > This is actually a bug in Darwin in that it doesn't parse the address > space part of the "ranges" property for the /pci node. OpenBIOS adds all > 3 spaces to the "ranges" - configuration, I/O and memory compared to > real Macs which only add I/O and memory. Which is generally fine because > each entry contains a 2-bit indicator so that the OS knows which entry > represents each address space. > > Except that Darwin evidently doesn't bother parsing the address space > from each "range" entry and blindly assumes that entry 0 is I/O space > and entry 1 is memory space as would be found in a real Mac device tree. > The result of this is that currently I/O accesses go to entry 0 (config > space) and memory accesses go to entry 1 (I/O space) which is why the > RTL8139 happens to work with your patch to force bus mastering. > > AFAICT with basic testing here, forcing the /pci node to mimic that of a > real Mac by omitting the configuration space entry seems to fix all PCI > accesses (or at least they get routed to the correct address spaces), I > see an attempt to set the bus master bit in the command register, and I > don't get a reset timeout on startup for the RTL8139 anymore. Wow, you figure out stuff fast! Will be waiting for your patches.
On 02/01/16 17:08, Programmingkid wrote: >> After some head scratching chasing this through for several hours >> yesterday, I've worked out what is happening and have a patchset for >> OpenBIOS which should fix the issue (will post later along with some >> other PCI fixes for testing). >> >> This is actually a bug in Darwin in that it doesn't parse the address >> space part of the "ranges" property for the /pci node. OpenBIOS adds all >> 3 spaces to the "ranges" - configuration, I/O and memory compared to >> real Macs which only add I/O and memory. Which is generally fine because >> each entry contains a 2-bit indicator so that the OS knows which entry >> represents each address space. >> >> Except that Darwin evidently doesn't bother parsing the address space >> from each "range" entry and blindly assumes that entry 0 is I/O space >> and entry 1 is memory space as would be found in a real Mac device tree. >> The result of this is that currently I/O accesses go to entry 0 (config >> space) and memory accesses go to entry 1 (I/O space) which is why the >> RTL8139 happens to work with your patch to force bus mastering. >> >> AFAICT with basic testing here, forcing the /pci node to mimic that of a >> real Mac by omitting the configuration space entry seems to fix all PCI >> accesses (or at least they get routed to the correct address spaces), I >> see an attempt to set the bus master bit in the command register, and I >> don't get a reset timeout on startup for the RTL8139 anymore. > > > Wow, you figure out stuff fast! Will be waiting for your patches. I've just posted the OpenBIOS patches to the list (see http://www.openfirmware.info/pipermail/openbios/2016-January/008959.html). Please test and let me know if this solves your networking problems against a vanilla QEMU git master checkout - it looks good in the logs, but I can't quite figure out how to manually bring up a new en0 interface in Darwin from the command line. ATB, Mark.
diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c index 90ef72b..e17a471 100644 --- a/hw/net/rtl8139.c +++ b/hw/net/rtl8139.c @@ -64,6 +64,14 @@ /* debug RTL8139 card */ //#define DEBUG_RTL8139 1 +/* + * The driver that ships with Mac OS 10.4 has to have its base address register + * 0 set to MMIO space. This directly conflicts with the Linux driver that + * needs the PIO set to base address register 0. Mac OS 10.5 or higher does not + * need this macro set. + */ +/* #define MAC_OS_10_4_DRIVER_COMPATIBILITY 1 */ + #define PCI_PERIOD 30 /* 30 ns period = 33.333333 Mhz frequency */ #define SET_MASKED(input, mask, curr) \ @@ -3444,9 +3452,14 @@ static void pci_rtl8139_realize(PCIDevice *dev, Error **errp) "rtl8139", 0x100); memory_region_init_io(&s->bar_mem, OBJECT(s), &rtl8139_mmio_ops, s, "rtl8139", 0x100); + +#ifdef MAC_OS_10_4_DRIVER_COMPATIBILITY + pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->bar_mem); + pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->bar_io); +#else pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->bar_io); pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->bar_mem); - +#endif qemu_macaddr_default_if_unset(&s->conf.macaddr); /* prepare eeprom */ diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index 379b6e1..8e95c75 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -699,6 +699,7 @@ static inline int pci_dma_rw(PCIDevice *dev, dma_addr_t addr, static inline int pci_dma_read(PCIDevice *dev, dma_addr_t addr, void *buf, dma_addr_t len) { + memory_region_set_enabled(&dev->bus_master_enable_region, true); return pci_dma_rw(dev, addr, buf, len, DMA_DIRECTION_TO_DEVICE); }
This patch solves the few problems that needed to be solved in order for a Mac OS 10.4 guest to use the rtl8139 nic. The pci_dma_read() function would only return zero when a MemoryRegion object was not enabled. Enabling the object makes the pci_dma_read() function work correctly. The Linux rtl8139 driver needs base address register zero set to PIO. This conflicts with Mac OS 10.4's driver needing base address register zero set to MMIO. So a macro has been added that allows the user to decide which operating system this network interface card will be compatible with. Note: Windows XP's driver works regardless of the macro setting. Signed-off-by: John Arbuckle <programmingkidx@gmail.com> --- hw/net/rtl8139.c | 15 ++++++++++++++- include/hw/pci/pci.h | 1 + 2 files changed, 15 insertions(+), 1 deletions(-)