From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony Liguori Subject: Re: [PATCH] e1000 VLAN offload emulation Date: Thu, 20 Nov 2008 17:11:35 -0600 Message-ID: <4925EEA7.6000804@codemonkey.ws> References: <1227127052.7652.20.camel@lappy> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: qemu-devel , kvm To: Alex Williamson Return-path: Received: from nf-out-0910.google.com ([64.233.182.190]:10720 "EHLO nf-out-0910.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751187AbYKTXLm (ORCPT ); Thu, 20 Nov 2008 18:11:42 -0500 Received: by nf-out-0910.google.com with SMTP id d3so359329nfc.21 for ; Thu, 20 Nov 2008 15:11:40 -0800 (PST) In-Reply-To: <1227127052.7652.20.camel@lappy> Sender: kvm-owner@vger.kernel.org List-ID: Alex Williamson wrote: > We're currently ignoring the e1000 VLAN tagging, stripping and filtering > features in the e1000 emulation. This patch adds backing for the > relevant registers and provides a software implementation of the > acceleration, such that a guest can make use of VLANs. > > This is mostly (only?) useful for a guest on a bridge (not user mode > networking). The only caveat beyond that is that you need to make sure > the host NIC isn't doing it's own tagging, stripping, or filtering. > This generally means the host NIC on the bridge should not be part of a > VLAN. Thanks, > > Alex > > Signed-off-by: Alex Williamson > -- > > Patch against KVM, but applies w/ only one line of offset to qemu. > Hi Alex, The patch looks good but I don't like the idea of taking patches against kvm-userspace. It suggests that people have only tested in kvm-userspace which means it's not guaranteed to work (or even compile) in upstream QEMU. Please rebase against QEMU and resubmit. Regards, Anthony Liguori > diff --git a/qemu/hw/e1000.c b/qemu/hw/e1000.c > index e2cc7f1..7be87e4 100644 > --- a/qemu/hw/e1000.c > +++ b/qemu/hw/e1000.c > @@ -89,9 +89,12 @@ typedef struct E1000State_st { > int check_rxov; > struct e1000_tx { > unsigned char header[256]; > + unsigned char vlan_header[4]; > + unsigned char vlan[4]; > unsigned char data[0x10000]; > uint16_t size; > unsigned char sum_needed; > + unsigned char vlan_needed; > uint8_t ipcss; > uint8_t ipcso; > uint16_t ipcse; > @@ -128,7 +131,8 @@ enum { > defreg(TDBAL), defreg(TDH), defreg(TDLEN), defreg(TDT), > defreg(TORH), defreg(TORL), defreg(TOTH), defreg(TOTL), > defreg(TPR), defreg(TPT), defreg(TXDCTL), defreg(WUFC), > - defreg(RA), defreg(MTA), defreg(CRCERRS), > + defreg(RA), defreg(MTA), defreg(CRCERRS),defreg(VFTA), > + defreg(VET), > }; > > enum { PHY_R = 1, PHY_W = 2, PHY_RW = PHY_R | PHY_W }; > @@ -294,6 +298,31 @@ putsum(uint8_t *data, uint32_t n, uint32_t sloc, uint32_t css, uint32_t cse) > } > } > > +static inline int > +vlan_enabled(E1000State *s) > +{ > + return ((s->mac_reg[CTRL] & E1000_CTRL_VME) != 0); > +} > + > +static inline int > +vlan_rx_filter_enabled(E1000State *s) > +{ > + return ((s->mac_reg[RCTL] & E1000_RCTL_VFE) != 0); > +} > + > +static inline int > +is_vlan_packet(E1000State *s, const uint8_t *buf) > +{ > + return (be16_to_cpup((uint16_t *)(buf + 12)) == > + le16_to_cpup((uint16_t *)(s->mac_reg + VET))); > +} > + > +static inline int > +is_vlan_txd(uint32_t txd_lower) > +{ > + return ((txd_lower & E1000_TXD_CMD_VLE) != 0); > +} > + > static void > xmit_seg(E1000State *s) > { > @@ -336,7 +365,12 @@ xmit_seg(E1000State *s) > putsum(tp->data, tp->size, tp->tucso, tp->tucss, tp->tucse); > if (tp->sum_needed & E1000_TXD_POPTS_IXSM) > putsum(tp->data, tp->size, tp->ipcso, tp->ipcss, tp->ipcse); > - qemu_send_packet(s->vc, tp->data, tp->size); > + if (tp->vlan_needed) { > + memmove(tp->vlan, tp->data, 12); > + memcpy(tp->data + 8, tp->vlan_header, 4); > + qemu_send_packet(s->vc, tp->vlan, tp->size + 4); > + } else > + qemu_send_packet(s->vc, tp->data, tp->size); > s->mac_reg[TPT]++; > s->mac_reg[GPTC]++; > n = s->mac_reg[TOTL]; > @@ -383,6 +417,15 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp) > // legacy descriptor > tp->cptse = 0; > > + if (vlan_enabled(s) && is_vlan_txd(txd_lower) && > + (tp->cptse || txd_lower & E1000_TXD_CMD_EOP)) { > + tp->vlan_needed = 1; > + cpu_to_be16wu((uint16_t *)(tp->vlan_header), > + le16_to_cpup((uint16_t *)(s->mac_reg + VET))); > + cpu_to_be16wu((uint16_t *)(tp->vlan_header + 2), > + le16_to_cpu(dp->upper.fields.special)); > + } > + > addr = le64_to_cpu(dp->buffer_addr); > if (tp->tse && tp->cptse) { > hdr = tp->hdr_len; > @@ -416,6 +459,7 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp) > xmit_seg(s); > tp->tso_frames = 0; > tp->sum_needed = 0; > + tp->vlan_needed = 0; > tp->size = 0; > tp->cptse = 0; > } > @@ -482,6 +526,14 @@ receive_filter(E1000State *s, const uint8_t *buf, int size) > static int mta_shift[] = {4, 3, 2, 0}; > uint32_t f, rctl = s->mac_reg[RCTL], ra[2], *rp; > > + if (is_vlan_packet(s, buf) && vlan_rx_filter_enabled(s)) { > + uint16_t vid = be16_to_cpup((uint16_t *)(buf + 14)); > + uint32_t vfta = le32_to_cpup((uint32_t *)(s->mac_reg + VFTA) + > + ((vid >> 5) & 0x7f)); > + if ((vfta & (1 << (vid & 0x1f))) == 0) > + return 0; > + } > + > if (rctl & E1000_RCTL_UPE) // promiscuous > return 1; > > @@ -536,6 +588,8 @@ e1000_receive(void *opaque, const uint8_t *buf, int size) > target_phys_addr_t base; > unsigned int n, rdt; > uint32_t rdh_start; > + uint16_t vlan_special = 0; > + uint8_t vlan_status = 0, vlan_offset = 0; > > if (!(s->mac_reg[RCTL] & E1000_RCTL_EN)) > return; > @@ -549,6 +603,14 @@ e1000_receive(void *opaque, const uint8_t *buf, int size) > if (!receive_filter(s, buf, size)) > return; > > + if (vlan_enabled(s) && is_vlan_packet(s, buf)) { > + vlan_special = cpu_to_le16(be16_to_cpup((uint16_t *)(buf + 14))); > + memmove((void *)(buf + 4), buf, 12); > + vlan_status = E1000_RXD_STAT_VP; > + vlan_offset = 4; > + size -= 4; > + } > + > rdh_start = s->mac_reg[RDH]; > size += 4; // for the header > do { > @@ -559,10 +621,11 @@ e1000_receive(void *opaque, const uint8_t *buf, int size) > base = ((uint64_t)s->mac_reg[RDBAH] << 32) + s->mac_reg[RDBAL] + > sizeof(desc) * s->mac_reg[RDH]; > cpu_physical_memory_read(base, (void *)&desc, sizeof(desc)); > - desc.status |= E1000_RXD_STAT_DD; > + desc.special = vlan_special; > + desc.status |= (vlan_status | E1000_RXD_STAT_DD); > if (desc.buffer_addr) { > cpu_physical_memory_write(le64_to_cpu(desc.buffer_addr), > - (void *)buf, size); > + (void *)(buf + vlan_offset), size); > desc.length = cpu_to_le16(size); > desc.status |= E1000_RXD_STAT_EOP|E1000_RXD_STAT_IXSM; > } else // as per intel docs; skip descriptors with null buf addr > @@ -692,7 +755,7 @@ static uint32_t (*macreg_readops[])(E1000State *, int) = { > getreg(WUFC), getreg(TDT), getreg(CTRL), getreg(LEDCTL), > getreg(MANC), getreg(MDIC), getreg(SWSM), getreg(STATUS), > getreg(TORL), getreg(TOTL), getreg(IMS), getreg(TCTL), > - getreg(RDH), getreg(RDT), > + getreg(RDH), getreg(RDT), getreg(VET), > > [TOTH] = mac_read_clr8, [TORH] = mac_read_clr8, [GPRC] = mac_read_clr4, > [GPTC] = mac_read_clr4, [TPR] = mac_read_clr4, [TPT] = mac_read_clr4, > @@ -700,6 +763,7 @@ static uint32_t (*macreg_readops[])(E1000State *, int) = { > [CRCERRS ... MPC] = &mac_readreg, > [RA ... RA+31] = &mac_readreg, > [MTA ... MTA+127] = &mac_readreg, > + [VFTA ... VFTA+127] = &mac_readreg, > }; > enum { NREADOPS = sizeof(macreg_readops) / sizeof(*macreg_readops) }; > > @@ -707,7 +771,7 @@ enum { NREADOPS = sizeof(macreg_readops) / sizeof(*macreg_readops) }; > static void (*macreg_writeops[])(E1000State *, int, uint32_t) = { > putreg(PBA), putreg(EERD), putreg(SWSM), putreg(WUFC), > putreg(TDBAL), putreg(TDBAH), putreg(TXDCTL), putreg(RDBAH), > - putreg(RDBAL), putreg(LEDCTL), > + putreg(RDBAL), putreg(LEDCTL), putreg(CTRL), putreg(VET), > [TDLEN] = set_dlen, [RDLEN] = set_dlen, [TCTL] = set_tctl, > [TDT] = set_tctl, [MDIC] = set_mdic, [ICS] = set_ics, > [TDH] = set_16bit, [RDH] = set_16bit, [RDT] = set_rdt, > @@ -715,6 +779,7 @@ static void (*macreg_writeops[])(E1000State *, int, uint32_t) = { > [EECD] = set_eecd, [RCTL] = set_rx_control, > [RA ... RA+31] = &mac_writereg, > [MTA ... MTA+127] = &mac_writereg, > + [VFTA ... VFTA+127] = &mac_writereg, > }; > enum { NWRITEOPS = sizeof(macreg_writeops) / sizeof(*macreg_writeops) }; > > @@ -789,13 +854,14 @@ static const int mac_regtosave[] = { > LEDCTL, MANC, MDIC, MPC, PBA, RCTL, RDBAH, RDBAL, RDH, > RDLEN, RDT, STATUS, SWSM, TCTL, TDBAH, TDBAL, TDH, TDLEN, > TDT, TORH, TORL, TOTH, TOTL, TPR, TPT, TXDCTL, WUFC, > + VET, > }; > enum { MAC_NSAVE = sizeof mac_regtosave/sizeof *mac_regtosave }; > > static const struct { > int size; > int array0; > -} mac_regarraystosave[] = { {32, RA}, {128, MTA} }; > +} mac_regarraystosave[] = { {32, RA}, {128, MTA}, {128, VFTA} }; > enum { MAC_NARRAYS = sizeof mac_regarraystosave/sizeof *mac_regarraystosave }; > > static void > > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >