All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
To: "Philippe Mathieu-Daudé" <f4bug@amsat.org>, qemu-devel@nongnu.org
Cc: Jason Wang <jasowang@redhat.com>,
	Laurent Vivier <laurent@vivier.eu>,
	Finn Thain <fthain@linux-m68k.org>
Subject: Re: [RFC PATCH 4/6] dp8393x: Store CAM registers as 16-bit
Date: Tue, 6 Jul 2021 20:27:10 +0100	[thread overview]
Message-ID: <f3f0d3e2-88ea-467d-caa4-aa1ac91c6545@ilande.co.uk> (raw)
In-Reply-To: <203e8c6c-94ac-e93b-8fa3-1ea64e9195e5@amsat.org>

On 06/07/2021 18:29, Philippe Mathieu-Daudé wrote:

> On 7/4/21 4:48 PM, Mark Cave-Ayland wrote:
>> On 03/07/2021 15:19, Philippe Mathieu-Daudé wrote:
>>
>>> Per the DP83932C datasheet from July 1995:
>>>
>>>     4.0 SONIC Registers
>>>     4.1 THE CAM UNIT
>>>
>>>       The Content Addressable Memory (CAM) consists of sixteen
>>>       48-bit entries for complete address filtering of network
>>>       packets. Each entry corresponds to a 48-bit destination
>>>       address that is user programmable and can contain any
>>>       combination of Multicast or Physical addresses. Each entry
>>>       is partitioned into three 16-bit CAM cells accessible
>>>       through CAM Address Ports (CAP 2, CAP 1 and CAP 0) with
>>>       CAP0 corresponding to the least significant 16 bits of
>>>       the Destination Address and CAP2 corresponding to the
>>>       most significant bits.
>>>
>>> Store the CAM registers as 16-bit as it simplifies the code.
>>> There is no change in the migration stream.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>>    hw/net/dp8393x.c | 23 ++++++++++-------------
>>>    1 file changed, 10 insertions(+), 13 deletions(-)
> 
>>> @@ -987,7 +984,7 @@ static const VMStateDescription vmstate_dp8393x = {
>>>        .version_id = 0,
>>>        .minimum_version_id = 0,
>>>        .fields = (VMStateField []) {
>>> -        VMSTATE_BUFFER_UNSAFE(cam, dp8393xState, 0, 16 * 6),
>>> +        VMSTATE_BUFFER_UNSAFE(cam, dp8393xState, 0, 16 * 3 * 2),
>>>            VMSTATE_UINT16_ARRAY(regs, dp8393xState, 0x40),
>>>            VMSTATE_END_OF_LIST()
>>>        }
>>
>> I'd still be inclined to change VMSTATE_BUFFER_UNSAFE for
>> VMSTATE_UINT16_ARRAY whilst you can do it without having to worry about
>> the migration stream being already broken, but anyhow:
>>
>> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> 
> Do you want me to squash:
> 
> -- >8 --
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index 1d1837dbd38..4c2fa0aabbd 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -951,10 +951,10 @@ static void dp8393x_realize(DeviceState *dev,
> Error **errp)
> 
>   static const VMStateDescription vmstate_dp8393x = {
>       .name = "dp8393x",
> -    .version_id = 0,
> -    .minimum_version_id = 0,
> +    .version_id = 1,
> +    .minimum_version_id = 1,
>       .fields = (VMStateField []) {
> -        VMSTATE_BUFFER_UNSAFE(cam, dp8393xState, 0, 16 * 3 * 2),
> +        VMSTATE_UINT16_ARRAY(cam, dp8393xState, 0, 16 * 3),
>           VMSTATE_UINT16_ARRAY(regs, dp8393xState, 0x40),
>           VMSTATE_END_OF_LIST()
>       }
> ---
> 
> Or send it as a new patch?

I don't mind either way. I think VMSTATE_UINT16_ARRAY is nicer, and it's very rare 
that you get the freedom to make a migration change like this without having to worry 
about compatibility. It's also really quick and easy to test.

If it passes your local tests and you would prefer to squash rather than re-post then 
you can also add a:

Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

to the patchset. I included the list of guests I tested in the cover note, but forgot 
to explicitly add the T-b tag.


ATB,

Mark.


  reply	other threads:[~2021-07-06 19:28 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-03 14:19 [RFC PATCH 0/6] dp8393x: Housekeeping Philippe Mathieu-Daudé
2021-07-03 14:19 ` [PATCH 1/6] dp8393x: fix CAM descriptor entry index Philippe Mathieu-Daudé
2021-07-03 14:19 ` [PATCH 2/6] dp8393x: don't force 32-bit register access Philippe Mathieu-Daudé
2021-07-03 14:39   ` Mark Cave-Ayland
2021-07-03 16:29     ` Philippe Mathieu-Daudé
2021-07-04 15:34       ` Mark Cave-Ayland
2021-07-04  2:06   ` Finn Thain
2021-07-03 14:19 ` [RFC PATCH 3/6] dp8393x: Restrict bus access to 16/32-bit operations Philippe Mathieu-Daudé
2021-07-03 14:52   ` Mark Cave-Ayland
2021-07-04 14:45   ` Mark Cave-Ayland
2021-07-03 14:19 ` [RFC PATCH 4/6] dp8393x: Store CAM registers as 16-bit Philippe Mathieu-Daudé
2021-07-03 14:56   ` Mark Cave-Ayland
2021-07-04 14:48   ` Mark Cave-Ayland
2021-07-06 17:29     ` Philippe Mathieu-Daudé
2021-07-06 19:27       ` Mark Cave-Ayland [this message]
2021-07-03 14:19 ` [PATCH 5/6] dp8393x: Replace address_space_rw(is_write=1) by address_space_write() Philippe Mathieu-Daudé
2021-07-03 14:57   ` Mark Cave-Ayland
2021-07-04 14:49   ` Mark Cave-Ayland
2021-07-03 14:19 ` [RFC PATCH 6/6] dp8393x: Rewrite dp8393x_get() / dp8393x_put() Philippe Mathieu-Daudé
2021-07-03 15:00   ` Mark Cave-Ayland
2021-07-03 15:04     ` Philippe Mathieu-Daudé
2021-07-04  1:46   ` Finn Thain
2021-07-04 15:07   ` Mark Cave-Ayland
2021-07-05  1:36     ` Finn Thain
2021-07-05  6:34       ` Mark Cave-Ayland
2021-07-07  1:30         ` Finn Thain
2021-07-07 10:12           ` Mark Cave-Ayland
2021-07-09  9:13             ` Finn Thain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f3f0d3e2-88ea-467d-caa4-aa1ac91c6545@ilande.co.uk \
    --to=mark.cave-ayland@ilande.co.uk \
    --cc=f4bug@amsat.org \
    --cc=fthain@linux-m68k.org \
    --cc=jasowang@redhat.com \
    --cc=laurent@vivier.eu \
    --cc=qemu-devel@nongnu.org \
    --subject='Re: [RFC PATCH 4/6] dp8393x: Store CAM registers as 16-bit' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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.