* [PATCH] hw/net/imx_fec: return 0xffff when accessing non-existing PHY
@ 2021-05-02 16:03 Guenter Roeck
2021-05-02 16:09 ` Bin Meng
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Guenter Roeck @ 2021-05-02 16:03 UTC (permalink / raw)
To: Peter Maydell
Cc: Jean-Christophe Dubois, Jason Wang, qemu-devel,
Philippe Mathieu-Daudé,
qemu-arm, Bin Meng, Guenter Roeck
If a PHY does not exist, attempts to read from it should return 0xffff.
Otherwise the Linux kernel will believe that a PHY is there and select
the non-existing PHY. This in turn will result in network errors later
on since the real PHY is not selected or configured.
Since reading from or writing to a non-existing PHY is not an emulation
error, replace guest error messages with traces.
Fixes: 461c51ad4275 ("Add a phy-num property to the i.MX FEC emulator")
Cc: Jean-Christophe Dubois <jcd@tribudubois.net>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
hw/net/imx_fec.c | 8 +++-----
hw/net/trace-events | 2 ++
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index f03450c028..9c7035bc94 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -283,9 +283,8 @@ static uint32_t imx_phy_read(IMXFECState *s, int reg)
uint32_t phy = reg / 32;
if (phy != s->phy_num) {
- qemu_log_mask(LOG_GUEST_ERROR, "[%s.phy]%s: Bad phy num %u\n",
- TYPE_IMX_FEC, __func__, phy);
- return 0;
+ trace_imx_phy_read_num(phy, s->phy_num);
+ return 0xffff;
}
reg %= 32;
@@ -345,8 +344,7 @@ static void imx_phy_write(IMXFECState *s, int reg, uint32_t val)
uint32_t phy = reg / 32;
if (phy != s->phy_num) {
- qemu_log_mask(LOG_GUEST_ERROR, "[%s.phy]%s: Bad phy num %u\n",
- TYPE_IMX_FEC, __func__, phy);
+ trace_imx_phy_write_num(phy, s->phy_num);
return;
}
diff --git a/hw/net/trace-events b/hw/net/trace-events
index baf25ffa7e..ee77238d9e 100644
--- a/hw/net/trace-events
+++ b/hw/net/trace-events
@@ -413,8 +413,10 @@ i82596_set_multicast(uint16_t count) "Added %d multicast entries"
i82596_channel_attention(void *s) "%p: Received CHANNEL ATTENTION"
# imx_fec.c
+imx_phy_read_num(int phy, int configured) "read request from unconfigured phy %d (configured %d)"
imx_phy_read(uint32_t val, int phy, int reg) "0x%04"PRIx32" <= phy[%d].reg[%d]"
imx_phy_write(uint32_t val, int phy, int reg) "0x%04"PRIx32" => phy[%d].reg[%d]"
+imx_phy_write_num(int phy, int configured) "write request to unconfigured phy %d (configured %d)"
imx_phy_update_link(const char *s) "%s"
imx_phy_reset(void) ""
imx_fec_read_bd(uint64_t addr, int flags, int len, int data) "tx_bd 0x%"PRIx64" flags 0x%04x len %d data 0x%08x"
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] hw/net/imx_fec: return 0xffff when accessing non-existing PHY
2021-05-02 16:03 [PATCH] hw/net/imx_fec: return 0xffff when accessing non-existing PHY Guenter Roeck
@ 2021-05-02 16:09 ` Bin Meng
2021-05-02 16:14 ` Guenter Roeck
2021-05-02 16:12 ` Philippe Mathieu-Daudé
2021-05-06 2:23 ` Jason Wang
2 siblings, 1 reply; 7+ messages in thread
From: Bin Meng @ 2021-05-02 16:09 UTC (permalink / raw)
To: Guenter Roeck
Cc: Peter Maydell, Jason Wang, qemu-devel@nongnu.org Developers,
Jean-Christophe Dubois, qemu-arm, Philippe Mathieu-Daudé
On Mon, May 3, 2021 at 12:03 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> If a PHY does not exist, attempts to read from it should return 0xffff.
> Otherwise the Linux kernel will believe that a PHY is there and select
> the non-existing PHY. This in turn will result in network errors later
> on since the real PHY is not selected or configured.
>
> Since reading from or writing to a non-existing PHY is not an emulation
> error, replace guest error messages with traces.
>
> Fixes: 461c51ad4275 ("Add a phy-num property to the i.MX FEC emulator")
> Cc: Jean-Christophe Dubois <jcd@tribudubois.net>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> hw/net/imx_fec.c | 8 +++-----
> hw/net/trace-events | 2 ++
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
> index f03450c028..9c7035bc94 100644
> --- a/hw/net/imx_fec.c
> +++ b/hw/net/imx_fec.c
> @@ -283,9 +283,8 @@ static uint32_t imx_phy_read(IMXFECState *s, int reg)
> uint32_t phy = reg / 32;
>
> if (phy != s->phy_num) {
> - qemu_log_mask(LOG_GUEST_ERROR, "[%s.phy]%s: Bad phy num %u\n",
> - TYPE_IMX_FEC, __func__, phy);
> - return 0;
> + trace_imx_phy_read_num(phy, s->phy_num);
> + return 0xffff;
> }
>
> reg %= 32;
> @@ -345,8 +344,7 @@ static void imx_phy_write(IMXFECState *s, int reg, uint32_t val)
> uint32_t phy = reg / 32;
>
> if (phy != s->phy_num) {
> - qemu_log_mask(LOG_GUEST_ERROR, "[%s.phy]%s: Bad phy num %u\n",
> - TYPE_IMX_FEC, __func__, phy);
> + trace_imx_phy_write_num(phy, s->phy_num);
> return;
> }
>
> diff --git a/hw/net/trace-events b/hw/net/trace-events
> index baf25ffa7e..ee77238d9e 100644
> --- a/hw/net/trace-events
> +++ b/hw/net/trace-events
> @@ -413,8 +413,10 @@ i82596_set_multicast(uint16_t count) "Added %d multicast entries"
> i82596_channel_attention(void *s) "%p: Received CHANNEL ATTENTION"
>
> # imx_fec.c
> +imx_phy_read_num(int phy, int configured) "read request from unconfigured phy %d (configured %d)"
nits: could we put this below imx_phy_read(), like you put
imx_phy_write_num after imx_phy_write?
> imx_phy_read(uint32_t val, int phy, int reg) "0x%04"PRIx32" <= phy[%d].reg[%d]"
> imx_phy_write(uint32_t val, int phy, int reg) "0x%04"PRIx32" => phy[%d].reg[%d]"
> +imx_phy_write_num(int phy, int configured) "write request to unconfigured phy %d (configured %d)"
> imx_phy_update_link(const char *s) "%s"
> imx_phy_reset(void) ""
> imx_fec_read_bd(uint64_t addr, int flags, int len, int data) "tx_bd 0x%"PRIx64" flags 0x%04x len %d data 0x%08x"
> --
Otherwise,
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Tested-by: Bin Meng <bmeng.cn@gmail.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] hw/net/imx_fec: return 0xffff when accessing non-existing PHY
2021-05-02 16:03 [PATCH] hw/net/imx_fec: return 0xffff when accessing non-existing PHY Guenter Roeck
2021-05-02 16:09 ` Bin Meng
@ 2021-05-02 16:12 ` Philippe Mathieu-Daudé
2021-05-06 2:23 ` Jason Wang
2 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-02 16:12 UTC (permalink / raw)
To: Guenter Roeck, Peter Maydell
Cc: Bin Meng, Jason Wang, qemu-arm, qemu-devel, Jean-Christophe Dubois
On 5/2/21 6:03 PM, Guenter Roeck wrote:
> If a PHY does not exist, attempts to read from it should return 0xffff.
> Otherwise the Linux kernel will believe that a PHY is there and select
> the non-existing PHY. This in turn will result in network errors later
> on since the real PHY is not selected or configured.
>
> Since reading from or writing to a non-existing PHY is not an emulation
> error, replace guest error messages with traces.
>
> Fixes: 461c51ad4275 ("Add a phy-num property to the i.MX FEC emulator")
> Cc: Jean-Christophe Dubois <jcd@tribudubois.net>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> hw/net/imx_fec.c | 8 +++-----
> hw/net/trace-events | 2 ++
> 2 files changed, 5 insertions(+), 5 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] hw/net/imx_fec: return 0xffff when accessing non-existing PHY
2021-05-02 16:09 ` Bin Meng
@ 2021-05-02 16:14 ` Guenter Roeck
2021-05-06 2:24 ` Jason Wang
0 siblings, 1 reply; 7+ messages in thread
From: Guenter Roeck @ 2021-05-02 16:14 UTC (permalink / raw)
To: Bin Meng
Cc: Peter Maydell, Jason Wang, qemu-devel@nongnu.org Developers,
Jean-Christophe Dubois, qemu-arm, Philippe Mathieu-Daudé
On 5/2/21 9:09 AM, Bin Meng wrote:
> On Mon, May 3, 2021 at 12:03 AM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> If a PHY does not exist, attempts to read from it should return 0xffff.
>> Otherwise the Linux kernel will believe that a PHY is there and select
>> the non-existing PHY. This in turn will result in network errors later
>> on since the real PHY is not selected or configured.
>>
>> Since reading from or writing to a non-existing PHY is not an emulation
>> error, replace guest error messages with traces.
>>
>> Fixes: 461c51ad4275 ("Add a phy-num property to the i.MX FEC emulator")
>> Cc: Jean-Christophe Dubois <jcd@tribudubois.net>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>> hw/net/imx_fec.c | 8 +++-----
>> hw/net/trace-events | 2 ++
>> 2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
>> index f03450c028..9c7035bc94 100644
>> --- a/hw/net/imx_fec.c
>> +++ b/hw/net/imx_fec.c
>> @@ -283,9 +283,8 @@ static uint32_t imx_phy_read(IMXFECState *s, int reg)
>> uint32_t phy = reg / 32;
>>
>> if (phy != s->phy_num) {
>> - qemu_log_mask(LOG_GUEST_ERROR, "[%s.phy]%s: Bad phy num %u\n",
>> - TYPE_IMX_FEC, __func__, phy);
>> - return 0;
>> + trace_imx_phy_read_num(phy, s->phy_num);
>> + return 0xffff;
>> }
>>
>> reg %= 32;
>> @@ -345,8 +344,7 @@ static void imx_phy_write(IMXFECState *s, int reg, uint32_t val)
>> uint32_t phy = reg / 32;
>>
>> if (phy != s->phy_num) {
>> - qemu_log_mask(LOG_GUEST_ERROR, "[%s.phy]%s: Bad phy num %u\n",
>> - TYPE_IMX_FEC, __func__, phy);
>> + trace_imx_phy_write_num(phy, s->phy_num);
>> return;
>> }
>>
>> diff --git a/hw/net/trace-events b/hw/net/trace-events
>> index baf25ffa7e..ee77238d9e 100644
>> --- a/hw/net/trace-events
>> +++ b/hw/net/trace-events
>> @@ -413,8 +413,10 @@ i82596_set_multicast(uint16_t count) "Added %d multicast entries"
>> i82596_channel_attention(void *s) "%p: Received CHANNEL ATTENTION"
>>
>> # imx_fec.c
>> +imx_phy_read_num(int phy, int configured) "read request from unconfigured phy %d (configured %d)"
>
> nits: could we put this below imx_phy_read(), like you put
> imx_phy_write_num after imx_phy_write?
>
Sure, makes sense. I'll wait a bit for other feedback and then resend.
>> imx_phy_read(uint32_t val, int phy, int reg) "0x%04"PRIx32" <= phy[%d].reg[%d]"
>> imx_phy_write(uint32_t val, int phy, int reg) "0x%04"PRIx32" => phy[%d].reg[%d]"
>> +imx_phy_write_num(int phy, int configured) "write request to unconfigured phy %d (configured %d)"
>> imx_phy_update_link(const char *s) "%s"
>> imx_phy_reset(void) ""
>> imx_fec_read_bd(uint64_t addr, int flags, int len, int data) "tx_bd 0x%"PRIx64" flags 0x%04x len %d data 0x%08x"
>> --
>
> Otherwise,
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> Tested-by: Bin Meng <bmeng.cn@gmail.com>
>
Thanks!
Guenter
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] hw/net/imx_fec: return 0xffff when accessing non-existing PHY
2021-05-02 16:03 [PATCH] hw/net/imx_fec: return 0xffff when accessing non-existing PHY Guenter Roeck
2021-05-02 16:09 ` Bin Meng
2021-05-02 16:12 ` Philippe Mathieu-Daudé
@ 2021-05-06 2:23 ` Jason Wang
2 siblings, 0 replies; 7+ messages in thread
From: Jason Wang @ 2021-05-06 2:23 UTC (permalink / raw)
To: Guenter Roeck, Peter Maydell
Cc: Jean-Christophe Dubois, Philippe Mathieu-Daudé,
qemu-arm, qemu-devel, Bin Meng
在 2021/5/3 上午12:03, Guenter Roeck 写道:
> If a PHY does not exist, attempts to read from it should return 0xffff.
> Otherwise the Linux kernel will believe that a PHY is there and select
> the non-existing PHY. This in turn will result in network errors later
> on since the real PHY is not selected or configured.
>
> Since reading from or writing to a non-existing PHY is not an emulation
> error, replace guest error messages with traces.
>
> Fixes: 461c51ad4275 ("Add a phy-num property to the i.MX FEC emulator")
> Cc: Jean-Christophe Dubois <jcd@tribudubois.net>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Applied.
Thanks
> ---
> hw/net/imx_fec.c | 8 +++-----
> hw/net/trace-events | 2 ++
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
> index f03450c028..9c7035bc94 100644
> --- a/hw/net/imx_fec.c
> +++ b/hw/net/imx_fec.c
> @@ -283,9 +283,8 @@ static uint32_t imx_phy_read(IMXFECState *s, int reg)
> uint32_t phy = reg / 32;
>
> if (phy != s->phy_num) {
> - qemu_log_mask(LOG_GUEST_ERROR, "[%s.phy]%s: Bad phy num %u\n",
> - TYPE_IMX_FEC, __func__, phy);
> - return 0;
> + trace_imx_phy_read_num(phy, s->phy_num);
> + return 0xffff;
> }
>
> reg %= 32;
> @@ -345,8 +344,7 @@ static void imx_phy_write(IMXFECState *s, int reg, uint32_t val)
> uint32_t phy = reg / 32;
>
> if (phy != s->phy_num) {
> - qemu_log_mask(LOG_GUEST_ERROR, "[%s.phy]%s: Bad phy num %u\n",
> - TYPE_IMX_FEC, __func__, phy);
> + trace_imx_phy_write_num(phy, s->phy_num);
> return;
> }
>
> diff --git a/hw/net/trace-events b/hw/net/trace-events
> index baf25ffa7e..ee77238d9e 100644
> --- a/hw/net/trace-events
> +++ b/hw/net/trace-events
> @@ -413,8 +413,10 @@ i82596_set_multicast(uint16_t count) "Added %d multicast entries"
> i82596_channel_attention(void *s) "%p: Received CHANNEL ATTENTION"
>
> # imx_fec.c
> +imx_phy_read_num(int phy, int configured) "read request from unconfigured phy %d (configured %d)"
> imx_phy_read(uint32_t val, int phy, int reg) "0x%04"PRIx32" <= phy[%d].reg[%d]"
> imx_phy_write(uint32_t val, int phy, int reg) "0x%04"PRIx32" => phy[%d].reg[%d]"
> +imx_phy_write_num(int phy, int configured) "write request to unconfigured phy %d (configured %d)"
> imx_phy_update_link(const char *s) "%s"
> imx_phy_reset(void) ""
> imx_fec_read_bd(uint64_t addr, int flags, int len, int data) "tx_bd 0x%"PRIx64" flags 0x%04x len %d data 0x%08x"
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] hw/net/imx_fec: return 0xffff when accessing non-existing PHY
2021-05-02 16:14 ` Guenter Roeck
@ 2021-05-06 2:24 ` Jason Wang
2021-05-06 3:04 ` Guenter Roeck
0 siblings, 1 reply; 7+ messages in thread
From: Jason Wang @ 2021-05-06 2:24 UTC (permalink / raw)
To: Guenter Roeck, Bin Meng
Cc: Jean-Christophe Dubois, Peter Maydell, qemu-arm,
qemu-devel@nongnu.org Developers, Philippe Mathieu-Daudé
在 2021/5/3 上午12:14, Guenter Roeck 写道:
> On 5/2/21 9:09 AM, Bin Meng wrote:
>> On Mon, May 3, 2021 at 12:03 AM Guenter Roeck <linux@roeck-us.net> wrote:
>>> If a PHY does not exist, attempts to read from it should return 0xffff.
>>> Otherwise the Linux kernel will believe that a PHY is there and select
>>> the non-existing PHY. This in turn will result in network errors later
>>> on since the real PHY is not selected or configured.
>>>
>>> Since reading from or writing to a non-existing PHY is not an emulation
>>> error, replace guest error messages with traces.
>>>
>>> Fixes: 461c51ad4275 ("Add a phy-num property to the i.MX FEC emulator")
>>> Cc: Jean-Christophe Dubois <jcd@tribudubois.net>
>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>> ---
>>> hw/net/imx_fec.c | 8 +++-----
>>> hw/net/trace-events | 2 ++
>>> 2 files changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
>>> index f03450c028..9c7035bc94 100644
>>> --- a/hw/net/imx_fec.c
>>> +++ b/hw/net/imx_fec.c
>>> @@ -283,9 +283,8 @@ static uint32_t imx_phy_read(IMXFECState *s, int reg)
>>> uint32_t phy = reg / 32;
>>>
>>> if (phy != s->phy_num) {
>>> - qemu_log_mask(LOG_GUEST_ERROR, "[%s.phy]%s: Bad phy num %u\n",
>>> - TYPE_IMX_FEC, __func__, phy);
>>> - return 0;
>>> + trace_imx_phy_read_num(phy, s->phy_num);
>>> + return 0xffff;
>>> }
>>>
>>> reg %= 32;
>>> @@ -345,8 +344,7 @@ static void imx_phy_write(IMXFECState *s, int reg, uint32_t val)
>>> uint32_t phy = reg / 32;
>>>
>>> if (phy != s->phy_num) {
>>> - qemu_log_mask(LOG_GUEST_ERROR, "[%s.phy]%s: Bad phy num %u\n",
>>> - TYPE_IMX_FEC, __func__, phy);
>>> + trace_imx_phy_write_num(phy, s->phy_num);
>>> return;
>>> }
>>>
>>> diff --git a/hw/net/trace-events b/hw/net/trace-events
>>> index baf25ffa7e..ee77238d9e 100644
>>> --- a/hw/net/trace-events
>>> +++ b/hw/net/trace-events
>>> @@ -413,8 +413,10 @@ i82596_set_multicast(uint16_t count) "Added %d multicast entries"
>>> i82596_channel_attention(void *s) "%p: Received CHANNEL ATTENTION"
>>>
>>> # imx_fec.c
>>> +imx_phy_read_num(int phy, int configured) "read request from unconfigured phy %d (configured %d)"
>> nits: could we put this below imx_phy_read(), like you put
>> imx_phy_write_num after imx_phy_write?
>>
> Sure, makes sense. I'll wait a bit for other feedback and then resend.
Ok, please send V2.
Thanks
>
>>> imx_phy_read(uint32_t val, int phy, int reg) "0x%04"PRIx32" <= phy[%d].reg[%d]"
>>> imx_phy_write(uint32_t val, int phy, int reg) "0x%04"PRIx32" => phy[%d].reg[%d]"
>>> +imx_phy_write_num(int phy, int configured) "write request to unconfigured phy %d (configured %d)"
>>> imx_phy_update_link(const char *s) "%s"
>>> imx_phy_reset(void) ""
>>> imx_fec_read_bd(uint64_t addr, int flags, int len, int data) "tx_bd 0x%"PRIx64" flags 0x%04x len %d data 0x%08x"
>>> --
>> Otherwise,
>> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
>> Tested-by: Bin Meng <bmeng.cn@gmail.com>
>>
> Thanks!
>
> Guenter
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] hw/net/imx_fec: return 0xffff when accessing non-existing PHY
2021-05-06 2:24 ` Jason Wang
@ 2021-05-06 3:04 ` Guenter Roeck
0 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2021-05-06 3:04 UTC (permalink / raw)
To: Jason Wang
Cc: Peter Maydell, qemu-devel@nongnu.org Developers,
Jean-Christophe Dubois, qemu-arm, Bin Meng,
Philippe Mathieu-Daudé
On Thu, May 06, 2021 at 10:24:52AM +0800, Jason Wang wrote:
[ ... ]
> Ok, please send V2.
>
You should have it by now. Please let me know if it got lost.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-05-06 3:06 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-02 16:03 [PATCH] hw/net/imx_fec: return 0xffff when accessing non-existing PHY Guenter Roeck
2021-05-02 16:09 ` Bin Meng
2021-05-02 16:14 ` Guenter Roeck
2021-05-06 2:24 ` Jason Wang
2021-05-06 3:04 ` Guenter Roeck
2021-05-02 16:12 ` Philippe Mathieu-Daudé
2021-05-06 2:23 ` Jason Wang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.