[PATCH] net: tulip: Set PCI revision to match dec21143

Marek Vasut posted 1 patch 4 years ago
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Test checkpatch passed
Test FreeBSD passed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200418002552.343480-1-marek.vasut+renesas@gmail.com
hw/net/tulip.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH] net: tulip: Set PCI revision to match dec21143
Posted by Marek Vasut 4 years ago
The tulip driver claims to emulate dec21143 and it does not emulate dec21142.
The dec21142 and dec21143 can be discerned by the PCI revision register,
where dec21142 reports value < 0x20 and dec21143 value >= 0x20. E.g. the
U-Boot 'tulip' driver also only supports dec21143 and verifies that the
PCI revision ID is >= 0x20, otherwise refuses to operate such a card.

This patch sets the PCI revision ID to 0x20 to match the dec21143 and
thus also permits e.g. U-Boot to work with the tulip emulation.

Fixes: 34ea023d4b95 ("net: add tulip (dec21143) driver")
Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Prasad J Pandit <pjp@fedoraproject.org>
Cc: Sven Schnelle <svens@stackframe.org>
---
 hw/net/tulip.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/net/tulip.c b/hw/net/tulip.c
index 1295f51d07..ffb6c2479a 100644
--- a/hw/net/tulip.c
+++ b/hw/net/tulip.c
@@ -962,6 +962,8 @@ static void pci_tulip_realize(PCIDevice *pci_dev, Error **errp)
 
     pci_conf = s->dev.config;
     pci_conf[PCI_INTERRUPT_PIN] = 1; /* interrupt pin A */
+    /* Anything with revision < 0x20 is DC21142, anything >= 0x20 is DC21143 */
+    pci_conf[PCI_REVISION_ID] = 0x20;
 
     s->eeprom = eeprom93xx_new(&pci_dev->qdev, 64);
     tulip_fill_eeprom(s);
-- 
2.25.1


Re: [PATCH] net: tulip: Set PCI revision to match dec21143
Posted by Marek Vasut 3 years, 11 months ago
On 4/18/20 2:25 AM, Marek Vasut wrote:
> The tulip driver claims to emulate dec21143 and it does not emulate dec21142.
> The dec21142 and dec21143 can be discerned by the PCI revision register,
> where dec21142 reports value < 0x20 and dec21143 value >= 0x20. E.g. the
> U-Boot 'tulip' driver also only supports dec21143 and verifies that the
> PCI revision ID is >= 0x20, otherwise refuses to operate such a card.
> 
> This patch sets the PCI revision ID to 0x20 to match the dec21143 and
> thus also permits e.g. U-Boot to work with the tulip emulation.
> 
> Fixes: 34ea023d4b95 ("net: add tulip (dec21143) driver")
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Prasad J Pandit <pjp@fedoraproject.org>
> Cc: Sven Schnelle <svens@stackframe.org>

Bump, any news on this ? This is a bugfix.

Re: [PATCH] net: tulip: Set PCI revision to match dec21143
Posted by Philippe Mathieu-Daudé 3 years, 10 months ago
Hi Sven, could you review thiw one-line patch?

On 4/18/20 2:25 AM, Marek Vasut wrote:
> The tulip driver claims to emulate dec21143 and it does not emulate dec21142.
> The dec21142 and dec21143 can be discerned by the PCI revision register,
> where dec21142 reports value < 0x20 and dec21143 value >= 0x20. E.g. the
> U-Boot 'tulip' driver also only supports dec21143 and verifies that the
> PCI revision ID is >= 0x20, otherwise refuses to operate such a card.
> 
> This patch sets the PCI revision ID to 0x20 to match the dec21143 and
> thus also permits e.g. U-Boot to work with the tulip emulation.
> 
> Fixes: 34ea023d4b95 ("net: add tulip (dec21143) driver")
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Prasad J Pandit <pjp@fedoraproject.org>
> Cc: Sven Schnelle <svens@stackframe.org>
> ---
>  hw/net/tulip.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/net/tulip.c b/hw/net/tulip.c
> index 1295f51d07..ffb6c2479a 100644
> --- a/hw/net/tulip.c
> +++ b/hw/net/tulip.c
> @@ -962,6 +962,8 @@ static void pci_tulip_realize(PCIDevice *pci_dev, Error **errp)
>  
>      pci_conf = s->dev.config;
>      pci_conf[PCI_INTERRUPT_PIN] = 1; /* interrupt pin A */
> +    /* Anything with revision < 0x20 is DC21142, anything >= 0x20 is DC21143 */
> +    pci_conf[PCI_REVISION_ID] = 0x20;
>  
>      s->eeprom = eeprom93xx_new(&pci_dev->qdev, 64);
>      tulip_fill_eeprom(s);
> 


Re: [PATCH] net: tulip: Set PCI revision to match dec21143
Posted by Sven Schnelle 3 years, 10 months ago
On Mon, Jun 08, 2020 at 12:17:11AM +0200, Philippe Mathieu-Daudé wrote:
> Hi Sven, could you review thiw one-line patch?
> 
> On 4/18/20 2:25 AM, Marek Vasut wrote:
> > The tulip driver claims to emulate dec21143 and it does not emulate dec21142.
> > The dec21142 and dec21143 can be discerned by the PCI revision register,
> > where dec21142 reports value < 0x20 and dec21143 value >= 0x20. E.g. the
> > U-Boot 'tulip' driver also only supports dec21143 and verifies that the
> > PCI revision ID is >= 0x20, otherwise refuses to operate such a card.
> > 
> > This patch sets the PCI revision ID to 0x20 to match the dec21143 and
> > thus also permits e.g. U-Boot to work with the tulip emulation.
> > 
> > Fixes: 34ea023d4b95 ("net: add tulip (dec21143) driver")
> > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> > Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Peter Maydell <peter.maydell@linaro.org>
> > Cc: Prasad J Pandit <pjp@fedoraproject.org>
> > Cc: Sven Schnelle <svens@stackframe.org>
> > ---
> >  hw/net/tulip.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/hw/net/tulip.c b/hw/net/tulip.c
> > index 1295f51d07..ffb6c2479a 100644
> > --- a/hw/net/tulip.c
> > +++ b/hw/net/tulip.c
> > @@ -962,6 +962,8 @@ static void pci_tulip_realize(PCIDevice *pci_dev, Error **errp)
> >  
> >      pci_conf = s->dev.config;
> >      pci_conf[PCI_INTERRUPT_PIN] = 1; /* interrupt pin A */
> > +    /* Anything with revision < 0x20 is DC21142, anything >= 0x20 is DC21143 */
> > +    pci_conf[PCI_REVISION_ID] = 0x20;
> >  
> >      s->eeprom = eeprom93xx_new(&pci_dev->qdev, 64);
> >      tulip_fill_eeprom(s);
> > 
> 

The intel datasheet for the DEC21143 lists only Rev IDs > 30 for this particular family:

21143-PB,TB,PC,TC - 0x30
21143-PD,TD - x041

but maybe older DEC chips used 0x20 - don't know. The most interesting question is
whether ancient OS' like HP-UX or Windows XP would still work with this patch, but
i don't have test images at hand right now.

Regards
Sven

Re: [PATCH] net: tulip: Set PCI revision to match dec21143
Posted by Philippe Mathieu-Daudé 3 years, 10 months ago
On 6/10/20 11:13 PM, Sven Schnelle wrote:
> On Mon, Jun 08, 2020 at 12:17:11AM +0200, Philippe Mathieu-Daudé wrote:
>> Hi Sven, could you review thiw one-line patch?
>>
>> On 4/18/20 2:25 AM, Marek Vasut wrote:
>>> The tulip driver claims to emulate dec21143 and it does not emulate dec21142.
>>> The dec21142 and dec21143 can be discerned by the PCI revision register,
>>> where dec21142 reports value < 0x20 and dec21143 value >= 0x20. E.g. the
>>> U-Boot 'tulip' driver also only supports dec21143 and verifies that the
>>> PCI revision ID is >= 0x20, otherwise refuses to operate such a card.
>>>
>>> This patch sets the PCI revision ID to 0x20 to match the dec21143 and
>>> thus also permits e.g. U-Boot to work with the tulip emulation.
>>>
>>> Fixes: 34ea023d4b95 ("net: add tulip (dec21143) driver")
>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>>> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: Peter Maydell <peter.maydell@linaro.org>
>>> Cc: Prasad J Pandit <pjp@fedoraproject.org>
>>> Cc: Sven Schnelle <svens@stackframe.org>
>>> ---
>>>  hw/net/tulip.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/hw/net/tulip.c b/hw/net/tulip.c
>>> index 1295f51d07..ffb6c2479a 100644
>>> --- a/hw/net/tulip.c
>>> +++ b/hw/net/tulip.c
>>> @@ -962,6 +962,8 @@ static void pci_tulip_realize(PCIDevice *pci_dev, Error **errp)
>>>  
>>>      pci_conf = s->dev.config;
>>>      pci_conf[PCI_INTERRUPT_PIN] = 1; /* interrupt pin A */
>>> +    /* Anything with revision < 0x20 is DC21142, anything >= 0x20 is DC21143 */
>>> +    pci_conf[PCI_REVISION_ID] = 0x20;
>>>  
>>>      s->eeprom = eeprom93xx_new(&pci_dev->qdev, 64);
>>>      tulip_fill_eeprom(s);
>>>
>>
> 
> The intel datasheet for the DEC21143 lists only Rev IDs > 30 for this particular family:
> 
> 21143-PB,TB,PC,TC - 0x30
> 21143-PD,TD - x041
> 
> but maybe older DEC chips used 0x20 - don't know. The most interesting question is
> whether ancient OS' like HP-UX or Windows XP would still work with this patch, but
> i don't have test images at hand right now.

So the question is whether your HP-UX/WinXP images also boot with a
DEC21142 (you aimed to model a DEC21143, and it is tested anyway).

Marek, suggestion:

Make pci_tulip_realize() abstract, add dec21142 and dec21143 models as
you suggested, making 'tulip' an alias of dec21142 for backward
compatibility. You can then use the dec21143.

Re: [PATCH] net: tulip: Set PCI revision to match dec21143
Posted by Marek Vasut 3 years, 10 months ago
On 6/11/20 12:27 AM, Philippe Mathieu-Daudé wrote:
> On 6/10/20 11:13 PM, Sven Schnelle wrote:
>> On Mon, Jun 08, 2020 at 12:17:11AM +0200, Philippe Mathieu-Daudé wrote:
>>> Hi Sven, could you review thiw one-line patch?
>>>
>>> On 4/18/20 2:25 AM, Marek Vasut wrote:
>>>> The tulip driver claims to emulate dec21143 and it does not emulate dec21142.
>>>> The dec21142 and dec21143 can be discerned by the PCI revision register,
>>>> where dec21142 reports value < 0x20 and dec21143 value >= 0x20. E.g. the
>>>> U-Boot 'tulip' driver also only supports dec21143 and verifies that the
>>>> PCI revision ID is >= 0x20, otherwise refuses to operate such a card.
>>>>
>>>> This patch sets the PCI revision ID to 0x20 to match the dec21143 and
>>>> thus also permits e.g. U-Boot to work with the tulip emulation.
>>>>
>>>> Fixes: 34ea023d4b95 ("net: add tulip (dec21143) driver")
>>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>>>> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>>> Cc: Peter Maydell <peter.maydell@linaro.org>
>>>> Cc: Prasad J Pandit <pjp@fedoraproject.org>
>>>> Cc: Sven Schnelle <svens@stackframe.org>
>>>> ---
>>>>  hw/net/tulip.c | 2 ++
>>>>  1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/hw/net/tulip.c b/hw/net/tulip.c
>>>> index 1295f51d07..ffb6c2479a 100644
>>>> --- a/hw/net/tulip.c
>>>> +++ b/hw/net/tulip.c
>>>> @@ -962,6 +962,8 @@ static void pci_tulip_realize(PCIDevice *pci_dev, Error **errp)
>>>>  
>>>>      pci_conf = s->dev.config;
>>>>      pci_conf[PCI_INTERRUPT_PIN] = 1; /* interrupt pin A */
>>>> +    /* Anything with revision < 0x20 is DC21142, anything >= 0x20 is DC21143 */
>>>> +    pci_conf[PCI_REVISION_ID] = 0x20;
>>>>  
>>>>      s->eeprom = eeprom93xx_new(&pci_dev->qdev, 64);
>>>>      tulip_fill_eeprom(s);
>>>>
>>>
>>
>> The intel datasheet for the DEC21143 lists only Rev IDs > 30 for this particular family:
>>
>> 21143-PB,TB,PC,TC - 0x30
>> 21143-PD,TD - x041
>>
>> but maybe older DEC chips used 0x20 - don't know. The most interesting question is
>> whether ancient OS' like HP-UX or Windows XP would still work with this patch, but
>> i don't have test images at hand right now.
> 
> So the question is whether your HP-UX/WinXP images also boot with a
> DEC21142 (you aimed to model a DEC21143, and it is tested anyway).
> 
> Marek, suggestion:
> 
> Make pci_tulip_realize() abstract, add dec21142 and dec21143 models as
> you suggested, making 'tulip' an alias of dec21142 for backward
> compatibility. You can then use the dec21143.

I don't have any way to test dec21142 , I only have dec21143 support in
U-Boot. U-Boot actually checks for this revision field and does not work
with dec21142 , so these older models must be somehow incompatible.
Hence, if we model only the dec21143 anyway, we should set that revision
ID to model it fully.