[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 3/8] e1000: Introduced an array to control th
From: |
Jason Wang |
Subject: |
Re: [Qemu-devel] [PATCH v5 3/8] e1000: Introduced an array to control the access to the MAC registers |
Date: |
Tue, 10 Nov 2015 13:37:16 +0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 11/09/2015 10:59 PM, Leonid Bloch wrote:
> The array of uint8_t's which is introduced here, contains useful metadata
> about the MAC registers: if a register should be always accessible, or if
> it is accessible, but partly implemented, or if the register requires a
> certain compatibility flag to be accessed. Currently, 5 hypothetical flags
> are supported (3 exist for e1000 so far) but if in the future more than 5
> flags will be needed, the datatype of this array can simply be swapped for
> a larger one.
>
> This patch is intended to solve the following current problems:
>
> 1) On migration between different versions of QEMU, which differ by the
> MAC registers implemented in them, some registers need not to be active if
> a compatibility flag is set, in order to preserve the machine's state
> perfectly for the older version. Checking this for each register
> individually, would create a lot of clutter in the code.
>
> 2) Some registers are (or may be) only partly implemented (e.g.
> placeholders that allow reading and writing, but lack other functions).
> In such cases it is better to print a debug warning on read/write attempts.
> As above, dealing with this functionality on a per-register level, would
> require longer and more messy code.
>
> Signed-off-by: Leonid Bloch <address@hidden>
> Signed-off-by: Dmitry Fleytman <address@hidden>
> ---
> hw/net/e1000.c | 85
> +++++++++++++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 73 insertions(+), 12 deletions(-)
>
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index 0e00afa..2bc533f 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -142,6 +142,8 @@ typedef struct E1000State_st {
> uint32_t compat_flags;
> } E1000State;
>
> +#define chkflag(x) (s->compat_flags & E1000_FLAG_##x)
> +
> typedef struct E1000BaseClass {
> PCIDeviceClass parent_class;
> uint16_t phy_id2;
> @@ -195,8 +197,7 @@ e1000_link_up(E1000State *s)
> static bool
> have_autoneg(E1000State *s)
> {
> - return (s->compat_flags & E1000_FLAG_AUTONEG) &&
> - (s->phy_reg[PHY_CTRL] & MII_CR_AUTO_NEG_EN);
> + return chkflag(AUTONEG) && (s->phy_reg[PHY_CTRL] & MII_CR_AUTO_NEG_EN);
> }
>
> static void
> @@ -321,7 +322,7 @@ set_interrupt_cause(E1000State *s, int index, uint32_t
> val)
> if (s->mit_timer_on) {
> return;
> }
> - if (s->compat_flags & E1000_FLAG_MIT) {
> + if (chkflag(MIT)) {
> /* Compute the next mitigation delay according to pending
> * interrupts and the current values of RADV (provided
> * RDTR!=0), TADV and ITR.
> @@ -1258,6 +1259,43 @@ static void (*macreg_writeops[])(E1000State *, int,
> uint32_t) = {
>
> enum { NWRITEOPS = ARRAY_SIZE(macreg_writeops) };
>
> +enum { MAC_ACCESS_ALWAYS = 1, MAC_ACCESS_PARTIAL = 2,
> + MAC_ACCESS_FLAG_NEEDED = 4 };
> +
> +#define markflag(x) ((E1000_FLAG_##x << 3) | MAC_ACCESS_FLAG_NEEDED)
> +/* In the array below the meaning of the bits is: [f|f|f|f|f|n|p|a]
> + * f - flag bits (up to 5 possible flags)
> + * n - flag needed
> + * p - partially implenented
> + * a - access enabled always
> + * n=p=a=0 - not implemented or unknown */
Looks like n=p=0 implies a=0? If yes we can probably get rid of bit 'a'
and save lots of lines below?
[...]
- [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation, Leonid Bloch, 2015/11/09
- [Qemu-devel] [PATCH v5 2/8] e1000: Add support for migrating the entire MAC registers' array, Leonid Bloch, 2015/11/09
- [Qemu-devel] [PATCH v5 1/8] e1000: Cosmetic and alignment fixes, Leonid Bloch, 2015/11/09
- [Qemu-devel] [PATCH v5 4/8] e1000: Trivial implementation of various MAC registers, Leonid Bloch, 2015/11/09
- [Qemu-devel] [PATCH v5 3/8] e1000: Introduced an array to control the access to the MAC registers, Leonid Bloch, 2015/11/09
- Re: [Qemu-devel] [PATCH v5 3/8] e1000: Introduced an array to control the access to the MAC registers,
Jason Wang <=
- [Qemu-devel] [PATCH v5 6/8] e1000: Fixing the received/transmitted octets' counters, Leonid Bloch, 2015/11/09
- [Qemu-devel] [PATCH v5 5/8] e1000: Fixing the received/transmitted packets' counters, Leonid Bloch, 2015/11/09
- [Qemu-devel] [PATCH v5 7/8] e1000: Fixing the packet address filtering procedure, Leonid Bloch, 2015/11/09
- [Qemu-devel] [PATCH v5 8/8] e1000: Implementing various counters, Leonid Bloch, 2015/11/09
- Re: [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation, Jason Wang, 2015/11/10