On Wed, Oct 21, 2015 at 01:58:19PM -0700, Alexander Duyck wrote:
> On 10/21/2015 09:37 AM, Lan Tianyu wrote:
> >In order to restore VF function after migration, add self emulation layer
> >to record regs' values during accessing regs.
> >
> >Signed-off-by: Lan Tianyu <tianyu.lan@xxxxxxxxx>
> >---
> > drivers/net/ethernet/intel/ixgbevf/Makefile | 3 ++-
> > drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 2 +-
> > .../net/ethernet/intel/ixgbevf/self-emulation.c | 26 ++++++++++++++++++++++
> > drivers/net/ethernet/intel/ixgbevf/vf.h | 5 ++++-
> > 4 files changed, 33 insertions(+), 3 deletions(-)
> > create mode 100644 drivers/net/ethernet/intel/ixgbevf/self-emulation.c
> >
> >diff --git a/drivers/net/ethernet/intel/ixgbevf/Makefile b/drivers/net/ethernet/intel/ixgbevf/Makefile
> >index 4ce4c97..841c884 100644
> >--- a/drivers/net/ethernet/intel/ixgbevf/Makefile
> >+++ b/drivers/net/ethernet/intel/ixgbevf/Makefile
> >@@ -31,7 +31,8 @@
> > obj-$(CONFIG_IXGBEVF) += ixgbevf.o
> >-ixgbevf-objs := vf.o \
> >+ixgbevf-objs := self-emulation.o \
> >+ vf.o \
> > mbx.o \
> > ethtool.o \
> > ixgbevf_main.o
> >diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> >index a16d267..4446916 100644
> >--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> >+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> >@@ -156,7 +156,7 @@ u32 ixgbevf_read_reg(struct ixgbe_hw *hw, u32 reg)
> > if (IXGBE_REMOVED(reg_addr))
> > return IXGBE_FAILED_READ_REG;
> >- value = readl(reg_addr + reg);
> >+ value = ixgbe_self_emul_readl(reg_addr, reg);
> > if (unlikely(value == IXGBE_FAILED_READ_REG))
> > ixgbevf_check_remove(hw, reg);
> > return value;
> >diff --git a/drivers/net/ethernet/intel/ixgbevf/self-emulation.c b/drivers/net/ethernet/intel/ixgbevf/self-emulation.c
> >new file mode 100644
> >index 0000000..d74b2da
> >--- /dev/null
> >+++ b/drivers/net/ethernet/intel/ixgbevf/self-emulation.c
> >@@ -0,0 +1,26 @@
> >+#include <linux/netdevice.h>
> >+#include <linux/pci.h>
> >+#include <linux/delay.h>
> >+#include <linux/interrupt.h>
> >+#include <net/arp.h>
> >+
> >+#include "vf.h"
> >+#include "ixgbevf.h"
> >+
> >+static u32 hw_regs[0x4000];
> >+
> >+u32 ixgbe_self_emul_readl(volatile void __iomem *base, u32 addr)
> >+{
> >+ u32 tmp;
> >+
> >+ tmp = readl(base + addr);
> >+ hw_regs[(unsigned long)addr] = tmp;
> >+
> >+ return tmp;
> >+}
> >+
> >+void ixgbe_self_emul_writel(u32 val, volatile void __iomem *base, u32 addr)
> >+{
> >+ hw_regs[(unsigned long)addr] = val;
> >+ writel(val, (volatile void __iomem *)(base + addr));
> >+}
>
> So I see what you are doing, however I don't think this adds much value.
> Many of the key registers for the device are not simple Read/Write
> registers. Most of them are things like write 1 to clear or some other sort
> of value where writing doesn't set the bit but has some other side effect.
> Just take a look through the Datasheet at registers such as the VFCTRL,
> VFMAILBOX, or most of the interrupt registers. The fact is simply storing
> the values off doesn't give you any real idea of what the state of things
> are.
It doesn't, but I guess the point is to isolate the migration-related logic
in the recovery code.
An alternative would be to have some smart logic all over the place to
only store what's required - that would be much more intrusive.
> >diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.h b/drivers/net/ethernet/intel/ixgbevf/vf.h
> >index d40f036..6a3f4eb 100644
> >--- a/drivers/net/ethernet/intel/ixgbevf/vf.h
> >+++ b/drivers/net/ethernet/intel/ixgbevf/vf.h
> >@@ -39,6 +39,9 @@
> > struct ixgbe_hw;
> >+u32 ixgbe_self_emul_readl(volatile void __iomem *base, u32 addr);
> >+void ixgbe_self_emul_writel(u32 val, volatile void __iomem *base, u32 addr);
> >+
> > /* iterator type for walking multicast address lists */
> > typedef u8* (*ixgbe_mc_addr_itr) (struct ixgbe_hw *hw, u8 **mc_addr_ptr,
> > u32 *vmdq);
> >@@ -182,7 +185,7 @@ static inline void ixgbe_write_reg(struct ixgbe_hw *hw, u32 reg, u32 value)
> > if (IXGBE_REMOVED(reg_addr))
> > return;
> >- writel(value, reg_addr + reg);
> >+ ixgbe_self_emul_writel(value, reg_addr, reg);
> > }
> > #define IXGBE_WRITE_REG(h, r, v) ixgbe_write_reg(h, r, v)
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
