Re: [Qemu-devel] [PATCH] Make rtl8139 network interface card compatible with Mac OS 10.4

[Mark Cave-Ayland]

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 <address@hidden>
>>>>
>>>> ---
>>>> 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.