[PATCH RESEND] net: convert xen-netfront to hw_features

[PATCH RESEND] net: convert xen-netfront to hw_features

[Michał Mirosław]

Not tested in any way. The original code for offload setting seems broken
as it resets the features on every netback reconnect.

This will set GSO_ROBUST at device creation time (earlier than connect time).

RX checksum offload is forced on - so advertise as it is.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
[I don't know Xen code enough to say this is correct. There is Xen netback
driver coming in, that has similar changes to be made. Please match
them up if you can.]

 drivers/net/xen-netfront.c |   57 +++++++++++++++++--------------------------
 1 files changed, 23 insertions(+), 34 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 5c8d9c3..2a71c9f 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1148,6 +1148,8 @@ static const struct net_device_ops xennet_netdev_ops = {
 	.ndo_change_mtu	     = xennet_change_mtu,
 	.ndo_set_mac_address = eth_mac_addr,
 	.ndo_validate_addr   = eth_validate_addr,
+	.ndo_fix_features    = xennet_fix_features,
+	.ndo_set_features    = xennet_set_features,
 };
 
 static struct net_device * __devinit xennet_create_dev(struct xenbus_device *dev)
@@ -1209,7 +1211,9 @@ static struct net_device * __devinit xennet_create_dev(struct xenbus_device *dev
 	netdev->netdev_ops	= &xennet_netdev_ops;
 
 	netif_napi_add(netdev, &np->napi, xennet_poll, 64);
-	netdev->features        = NETIF_F_IP_CSUM;
+	netdev->features        = NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
+				  NETIF_F_GSO_ROBUST;
+	netdev->hw_features	= NETIF_F_IP_CSUM | NETIF_F_SG | NETIF_F_TSO;
 
 	SET_ETHTOOL_OPS(netdev, &xennet_ethtool_ops);
 	SET_NETDEV_DEV(netdev, &dev->dev);
@@ -1510,52 +1514,40 @@ again:
 	return err;
 }
 
-static int xennet_set_sg(struct net_device *dev, u32 data)
+static u32 xennet_fix_features(struct net_device *dev, u32 features)
 {
-	if (data) {
-		struct netfront_info *np = netdev_priv(dev);
-		int val;
+	struct netfront_info *np = netdev_priv(dev);
+	int val;
 
+	if (features & NETIF_F_SG) {
 		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend, "feature-sg",
 				 "%d", &val) < 0)
 			val = 0;
+
 		if (!val)
-			return -ENOSYS;
-	} else if (dev->mtu > ETH_DATA_LEN)
-		dev->mtu = ETH_DATA_LEN;
-
-	return ethtool_op_set_sg(dev, data);
-}
-
-static int xennet_set_tso(struct net_device *dev, u32 data)
-{
-	if (data) {
-		struct netfront_info *np = netdev_priv(dev);
-		int val;
+			features &= ~NETIF_F_SG;
+	}
 
+	if (features & NETIF_F_TSO) {
 		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
 				 "feature-gso-tcpv4", "%d", &val) < 0)
 			val = 0;
+
 		if (!val)
-			return -ENOSYS;
+			features &= ~NETIF_F_TSO;
 	}
 
-	return ethtool_op_set_tso(dev, data);
+	return features;
 }
 
-static void xennet_set_features(struct net_device *dev)
+static int xennet_set_features(struct net_device *dev, u32 features)
 {
-	/* Turn off all GSO bits except ROBUST. */
-	dev->features &= ~NETIF_F_GSO_MASK;
-	dev->features |= NETIF_F_GSO_ROBUST;
-	xennet_set_sg(dev, 0);
+	if (!(features & NETIF_F_SG) && dev->mtu > ETH_DATA_LEN) {
+		netdev_info(dev, "Reducing MTU because no SG offload");
+		dev->mtu = ETH_DATA_LEN;
+	}
 
-	/* We need checksum offload to enable scatter/gather and TSO. */
-	if (!(dev->features & NETIF_F_IP_CSUM))
-		return;
-
-	if (!xennet_set_sg(dev, 1))
-		xennet_set_tso(dev, 1);
+	return 0;
 }
 
 static int xennet_connect(struct net_device *dev)
@@ -1582,7 +1574,7 @@ static int xennet_connect(struct net_device *dev)
 	if (err)
 		return err;
 
-	xennet_set_features(dev);
+	netdev_update_features(dev);
 
 	spin_lock_bh(&np->rx_lock);
 	spin_lock_irq(&np->tx_lock);
@@ -1710,9 +1702,6 @@ static void xennet_get_strings(struct net_device *dev, u32 stringset, u8 * data)
 
 static const struct ethtool_ops xennet_ethtool_ops =
 {
-	.set_tx_csum = ethtool_op_set_tx_csum,
-	.set_sg = xennet_set_sg,
-	.set_tso = xennet_set_tso,
 	.get_link = ethtool_op_get_link,
 
 	.get_sset_count = xennet_get_sset_count,
-- 
1.7.2.5

Re: [PATCH RESEND] net: convert xen-netfront to hw_features

[Ian Campbell]

On Thu, 2011-03-31 at 12:01 +0100, Michał Mirosław wrote:
> Not tested in any way. The original code for offload setting seems broken
> as it resets the features on every netback reconnect.

Thanks, I've got a pending TODO item to test this and propagate similar
changes to netback. I hope to get to it soon...

Is this urgent (for 2.6.39) IYHO? I think it's been broken this way for
a long time now...

Ian.

> 
> This will set GSO_ROBUST at device creation time (earlier than connect time).
> 
> RX checksum offload is forced on - so advertise as it is.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
> [I don't know Xen code enough to say this is correct. There is Xen netback
> driver coming in, that has similar changes to be made. Please match
> them up if you can.]
> 
>  drivers/net/xen-netfront.c |   57 +++++++++++++++++--------------------------
>  1 files changed, 23 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> index 5c8d9c3..2a71c9f 100644
> --- a/drivers/net/xen-netfront.c
> +++ b/drivers/net/xen-netfront.c
> @@ -1148,6 +1148,8 @@ static const struct net_device_ops xennet_netdev_ops = {
>  	.ndo_change_mtu	     = xennet_change_mtu,
>  	.ndo_set_mac_address = eth_mac_addr,
>  	.ndo_validate_addr   = eth_validate_addr,
> +	.ndo_fix_features    = xennet_fix_features,
> +	.ndo_set_features    = xennet_set_features,
>  };
>  
>  static struct net_device * __devinit xennet_create_dev(struct xenbus_device *dev)
> @@ -1209,7 +1211,9 @@ static struct net_device * __devinit xennet_create_dev(struct xenbus_device *dev
>  	netdev->netdev_ops	= &xennet_netdev_ops;
>  
>  	netif_napi_add(netdev, &np->napi, xennet_poll, 64);
> -	netdev->features        = NETIF_F_IP_CSUM;
> +	netdev->features        = NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
> +				  NETIF_F_GSO_ROBUST;
> +	netdev->hw_features	= NETIF_F_IP_CSUM | NETIF_F_SG | NETIF_F_TSO;
>  
>  	SET_ETHTOOL_OPS(netdev, &xennet_ethtool_ops);
>  	SET_NETDEV_DEV(netdev, &dev->dev);
> @@ -1510,52 +1514,40 @@ again:
>  	return err;
>  }
>  
> -static int xennet_set_sg(struct net_device *dev, u32 data)
> +static u32 xennet_fix_features(struct net_device *dev, u32 features)
>  {
> -	if (data) {
> -		struct netfront_info *np = netdev_priv(dev);
> -		int val;
> +	struct netfront_info *np = netdev_priv(dev);
> +	int val;
>  
> +	if (features & NETIF_F_SG) {
>  		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend, "feature-sg",
>  				 "%d", &val) < 0)
>  			val = 0;
> +
>  		if (!val)
> -			return -ENOSYS;
> -	} else if (dev->mtu > ETH_DATA_LEN)
> -		dev->mtu = ETH_DATA_LEN;
> -
> -	return ethtool_op_set_sg(dev, data);
> -}
> -
> -static int xennet_set_tso(struct net_device *dev, u32 data)
> -{
> -	if (data) {
> -		struct netfront_info *np = netdev_priv(dev);
> -		int val;
> +			features &= ~NETIF_F_SG;
> +	}
>  
> +	if (features & NETIF_F_TSO) {
>  		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
>  				 "feature-gso-tcpv4", "%d", &val) < 0)
>  			val = 0;
> +
>  		if (!val)
> -			return -ENOSYS;
> +			features &= ~NETIF_F_TSO;
>  	}
>  
> -	return ethtool_op_set_tso(dev, data);
> +	return features;
>  }
>  
> -static void xennet_set_features(struct net_device *dev)
> +static int xennet_set_features(struct net_device *dev, u32 features)
>  {
> -	/* Turn off all GSO bits except ROBUST. */
> -	dev->features &= ~NETIF_F_GSO_MASK;
> -	dev->features |= NETIF_F_GSO_ROBUST;
> -	xennet_set_sg(dev, 0);
> +	if (!(features & NETIF_F_SG) && dev->mtu > ETH_DATA_LEN) {
> +		netdev_info(dev, "Reducing MTU because no SG offload");
> +		dev->mtu = ETH_DATA_LEN;
> +	}
>  
> -	/* We need checksum offload to enable scatter/gather and TSO. */
> -	if (!(dev->features & NETIF_F_IP_CSUM))
> -		return;
> -
> -	if (!xennet_set_sg(dev, 1))
> -		xennet_set_tso(dev, 1);
> +	return 0;
>  }
>  
>  static int xennet_connect(struct net_device *dev)
> @@ -1582,7 +1574,7 @@ static int xennet_connect(struct net_device *dev)
>  	if (err)
>  		return err;
>  
> -	xennet_set_features(dev);
> +	netdev_update_features(dev);
>  
>  	spin_lock_bh(&np->rx_lock);
>  	spin_lock_irq(&np->tx_lock);
> @@ -1710,9 +1702,6 @@ static void xennet_get_strings(struct net_device *dev, u32 stringset, u8 * data)
>  
>  static const struct ethtool_ops xennet_ethtool_ops =
>  {
> -	.set_tx_csum = ethtool_op_set_tx_csum,
> -	.set_sg = xennet_set_sg,
> -	.set_tso = xennet_set_tso,
>  	.get_link = ethtool_op_get_link,
>  
>  	.get_sset_count = xennet_get_sset_count,


_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

Re: [PATCH RESEND] net: convert xen-netfront to hw_features

[Ian Campbell]

On Thu, 2011-03-31 at 12:01 +0100, Michał Mirosław wrote:
> Not tested in any way. The original code for offload setting seems broken
> as it resets the features on every netback reconnect.

Thanks, I've got a pending TODO item to test this and propagate similar
changes to netback. I hope to get to it soon...

Is this urgent (for 2.6.39) IYHO? I think it's been broken this way for
a long time now...

Ian.

> 
> This will set GSO_ROBUST at device creation time (earlier than connect time).
> 
> RX checksum offload is forced on - so advertise as it is.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
> [I don't know Xen code enough to say this is correct. There is Xen netback
> driver coming in, that has similar changes to be made. Please match
> them up if you can.]
> 
>  drivers/net/xen-netfront.c |   57 +++++++++++++++++--------------------------
>  1 files changed, 23 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> index 5c8d9c3..2a71c9f 100644
> --- a/drivers/net/xen-netfront.c
> +++ b/drivers/net/xen-netfront.c
> @@ -1148,6 +1148,8 @@ static const struct net_device_ops xennet_netdev_ops = {
>  	.ndo_change_mtu	     = xennet_change_mtu,
>  	.ndo_set_mac_address = eth_mac_addr,
>  	.ndo_validate_addr   = eth_validate_addr,
> +	.ndo_fix_features    = xennet_fix_features,
> +	.ndo_set_features    = xennet_set_features,
>  };
>  
>  static struct net_device * __devinit xennet_create_dev(struct xenbus_device *dev)
> @@ -1209,7 +1211,9 @@ static struct net_device * __devinit xennet_create_dev(struct xenbus_device *dev
>  	netdev->netdev_ops	= &xennet_netdev_ops;
>  
>  	netif_napi_add(netdev, &np->napi, xennet_poll, 64);
> -	netdev->features        = NETIF_F_IP_CSUM;
> +	netdev->features        = NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
> +				  NETIF_F_GSO_ROBUST;
> +	netdev->hw_features	= NETIF_F_IP_CSUM | NETIF_F_SG | NETIF_F_TSO;
>  
>  	SET_ETHTOOL_OPS(netdev, &xennet_ethtool_ops);
>  	SET_NETDEV_DEV(netdev, &dev->dev);
> @@ -1510,52 +1514,40 @@ again:
>  	return err;
>  }
>  
> -static int xennet_set_sg(struct net_device *dev, u32 data)
> +static u32 xennet_fix_features(struct net_device *dev, u32 features)
>  {
> -	if (data) {
> -		struct netfront_info *np = netdev_priv(dev);
> -		int val;
> +	struct netfront_info *np = netdev_priv(dev);
> +	int val;
>  
> +	if (features & NETIF_F_SG) {
>  		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend, "feature-sg",
>  				 "%d", &val) < 0)
>  			val = 0;
> +
>  		if (!val)
> -			return -ENOSYS;
> -	} else if (dev->mtu > ETH_DATA_LEN)
> -		dev->mtu = ETH_DATA_LEN;
> -
> -	return ethtool_op_set_sg(dev, data);
> -}
> -
> -static int xennet_set_tso(struct net_device *dev, u32 data)
> -{
> -	if (data) {
> -		struct netfront_info *np = netdev_priv(dev);
> -		int val;
> +			features &= ~NETIF_F_SG;
> +	}
>  
> +	if (features & NETIF_F_TSO) {
>  		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
>  				 "feature-gso-tcpv4", "%d", &val) < 0)
>  			val = 0;
> +
>  		if (!val)
> -			return -ENOSYS;
> +			features &= ~NETIF_F_TSO;
>  	}
>  
> -	return ethtool_op_set_tso(dev, data);
> +	return features;
>  }
>  
> -static void xennet_set_features(struct net_device *dev)
> +static int xennet_set_features(struct net_device *dev, u32 features)
>  {
> -	/* Turn off all GSO bits except ROBUST. */
> -	dev->features &= ~NETIF_F_GSO_MASK;
> -	dev->features |= NETIF_F_GSO_ROBUST;
> -	xennet_set_sg(dev, 0);
> +	if (!(features & NETIF_F_SG) && dev->mtu > ETH_DATA_LEN) {
> +		netdev_info(dev, "Reducing MTU because no SG offload");
> +		dev->mtu = ETH_DATA_LEN;
> +	}
>  
> -	/* We need checksum offload to enable scatter/gather and TSO. */
> -	if (!(dev->features & NETIF_F_IP_CSUM))
> -		return;
> -
> -	if (!xennet_set_sg(dev, 1))
> -		xennet_set_tso(dev, 1);
> +	return 0;
>  }
>  
>  static int xennet_connect(struct net_device *dev)
> @@ -1582,7 +1574,7 @@ static int xennet_connect(struct net_device *dev)
>  	if (err)
>  		return err;
>  
> -	xennet_set_features(dev);
> +	netdev_update_features(dev);
>  
>  	spin_lock_bh(&np->rx_lock);
>  	spin_lock_irq(&np->tx_lock);
> @@ -1710,9 +1702,6 @@ static void xennet_get_strings(struct net_device *dev, u32 stringset, u8 * data)
>  
>  static const struct ethtool_ops xennet_ethtool_ops =
>  {
> -	.set_tx_csum = ethtool_op_set_tx_csum,
> -	.set_sg = xennet_set_sg,
> -	.set_tso = xennet_set_tso,
>  	.get_link = ethtool_op_get_link,
>  
>  	.get_sset_count = xennet_get_sset_count,

Re: [PATCH RESEND] net: convert xen-netfront to hw_features

[Michał Mirosław]

On Thu, Mar 31, 2011 at 12:13:30PM +0100, Ian Campbell wrote:
> On Thu, 2011-03-31 at 12:01 +0100, Michał Mirosław wrote:
> > Not tested in any way. The original code for offload setting seems broken
> > as it resets the features on every netback reconnect.
> Thanks, I've got a pending TODO item to test this and propagate similar
> changes to netback. I hope to get to it soon...
> 
> Is this urgent (for 2.6.39) IYHO? I think it's been broken this way for
> a long time now...

This probably affects only people who are using suspending VM's to disk or
[live] migration. The bug is hard to notice if you're not looking for it.
I only noticed it after trying to change the code.

Best Regards,
Michał Mirosław

Re: [PATCH RESEND] net: convert xen-netfront to hw_features

[Michał Mirosław]

On Thu, Mar 31, 2011 at 12:13:30PM +0100, Ian Campbell wrote:
> On Thu, 2011-03-31 at 12:01 +0100, Michał Mirosław wrote:
> > Not tested in any way. The original code for offload setting seems broken
> > as it resets the features on every netback reconnect.
> Thanks, I've got a pending TODO item to test this and propagate similar
> changes to netback. I hope to get to it soon...
> 
> Is this urgent (for 2.6.39) IYHO? I think it's been broken this way for
> a long time now...

This probably affects only people who are using suspending VM's to disk or
[live] migration. The bug is hard to notice if you're not looking for it.
I only noticed it after trying to change the code.

Best Regards,
Michał Mirosław

Re: [PATCH RESEND] net: convert xen-netfront to hw_features

[David Miller]

From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Date: Thu, 31 Mar 2011 13:01:35 +0200 (CEST)

> Not tested in any way. The original code for offload setting seems broken
> as it resets the features on every netback reconnect.
> 
> This will set GSO_ROBUST at device creation time (earlier than connect time).
> 
> RX checksum offload is forced on - so advertise as it is.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

Applied.

[PATCH] xen: netfront: fix declaration order

[Eric Dumazet]

Le vendredi 01 avril 2011 à 20:54 -0700, David Miller a écrit :
> From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> Date: Thu, 31 Mar 2011 13:01:35 +0200 (CEST)
> 
> > Not tested in any way. The original code for offload setting seems broken
> > as it resets the features on every netback reconnect.
> > 
> > This will set GSO_ROBUST at device creation time (earlier than connect time).
> > 
> > RX checksum offload is forced on - so advertise as it is.
> > 
> > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> 
> Applied.

Hmm... I had to apply following patch to make it actually compile.

Thanks

[PATCH] xen: netfront: fix declaration order

Must declare xennet_fix_features() and xennet_set_features() before
using them.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/net/xen-netfront.c |   72 +++++++++++++++++------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index f6e7e27..0cfe4cc 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1140,6 +1140,42 @@ static void xennet_uninit(struct net_device *dev)
 	gnttab_free_grant_references(np->gref_rx_head);
 }
 
+static u32 xennet_fix_features(struct net_device *dev, u32 features)
+{
+	struct netfront_info *np = netdev_priv(dev);
+	int val;
+
+	if (features & NETIF_F_SG) {
+		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend, "feature-sg",
+				 "%d", &val) < 0)
+			val = 0;
+
+		if (!val)
+			features &= ~NETIF_F_SG;
+	}
+
+	if (features & NETIF_F_TSO) {
+		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
+				 "feature-gso-tcpv4", "%d", &val) < 0)
+			val = 0;
+
+		if (!val)
+			features &= ~NETIF_F_TSO;
+	}
+
+	return features;
+}
+
+static int xennet_set_features(struct net_device *dev, u32 features)
+{
+	if (!(features & NETIF_F_SG) && dev->mtu > ETH_DATA_LEN) {
+		netdev_info(dev, "Reducing MTU because no SG offload");
+		dev->mtu = ETH_DATA_LEN;
+	}
+
+	return 0;
+}
+
 static const struct net_device_ops xennet_netdev_ops = {
 	.ndo_open            = xennet_open,
 	.ndo_uninit          = xennet_uninit,
@@ -1513,42 +1549,6 @@ again:
 	return err;
 }
 
-static u32 xennet_fix_features(struct net_device *dev, u32 features)
-{
-	struct netfront_info *np = netdev_priv(dev);
-	int val;
-
-	if (features & NETIF_F_SG) {
-		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend, "feature-sg",
-				 "%d", &val) < 0)
-			val = 0;
-
-		if (!val)
-			features &= ~NETIF_F_SG;
-	}
-
-	if (features & NETIF_F_TSO) {
-		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
-				 "feature-gso-tcpv4", "%d", &val) < 0)
-			val = 0;
-
-		if (!val)
-			features &= ~NETIF_F_TSO;
-	}
-
-	return features;
-}
-
-static int xennet_set_features(struct net_device *dev, u32 features)
-{
-	if (!(features & NETIF_F_SG) && dev->mtu > ETH_DATA_LEN) {
-		netdev_info(dev, "Reducing MTU because no SG offload");
-		dev->mtu = ETH_DATA_LEN;
-	}
-
-	return 0;
-}
-
 static int xennet_connect(struct net_device *dev)
 {
 	struct netfront_info *np = netdev_priv(dev);

Re: [PATCH] xen: netfront: fix declaration order

[Michał Mirosław]

On Sun, Apr 03, 2011 at 01:07:19PM +0200, Eric Dumazet wrote:
> Le vendredi 01 avril 2011 à 20:54 -0700, David Miller a écrit :
> > From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > Date: Thu, 31 Mar 2011 13:01:35 +0200 (CEST)
> > 
> > > Not tested in any way. The original code for offload setting seems broken
> > > as it resets the features on every netback reconnect.
> > > 
> > > This will set GSO_ROBUST at device creation time (earlier than connect time).
> > > 
> > > RX checksum offload is forced on - so advertise as it is.
> > > 
> > > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > Applied.
> Hmm... I had to apply following patch to make it actually compile.

> [PATCH] xen: netfront: fix declaration order
> 
> Must declare xennet_fix_features() and xennet_set_features() before
> using them.

Hmm. Sorry for that. Looks like x86 allyesconfig doesn't include this
driver in the build. :/

There really needs to be something like CONFIG_LINT...

Best Regards,
Michał Mirosław

Re: [PATCH] xen: netfront: fix declaration order

[Michał Mirosław]

On Sun, Apr 03, 2011 at 01:07:19PM +0200, Eric Dumazet wrote:
> Le vendredi 01 avril 2011 à 20:54 -0700, David Miller a écrit :
> > From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > Date: Thu, 31 Mar 2011 13:01:35 +0200 (CEST)
> > 
> > > Not tested in any way. The original code for offload setting seems broken
> > > as it resets the features on every netback reconnect.
> > > 
> > > This will set GSO_ROBUST at device creation time (earlier than connect time).
> > > 
> > > RX checksum offload is forced on - so advertise as it is.
> > > 
> > > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > Applied.
> Hmm... I had to apply following patch to make it actually compile.

> [PATCH] xen: netfront: fix declaration order
> 
> Must declare xennet_fix_features() and xennet_set_features() before
> using them.

Hmm. Sorry for that. Looks like x86 allyesconfig doesn't include this
driver in the build. :/

There really needs to be something like CONFIG_LINT...

Best Regards,
Michał Mirosław

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

Re: [PATCH] xen: netfront: fix declaration order

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 03 Apr 2011 13:07:19 +0200

> [PATCH] xen: netfront: fix declaration order
> 
> Must declare xennet_fix_features() and xennet_set_features() before
> using them.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Michał Mirosław <mirq-linux@rere.qmqm.pl>

Ugh, it makes no sense that XEN won't make it into the x86_32
allmodconfig build.  Those dependencies in arch/x86/xen/Kconfig
are terrible.

For if it did, I would have caught this immediately.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

Re: [PATCH] xen: netfront: fix declaration order

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 03 Apr 2011 13:07:19 +0200

> [PATCH] xen: netfront: fix declaration order
> 
> Must declare xennet_fix_features() and xennet_set_features() before
> using them.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Michał Mirosław <mirq-linux@rere.qmqm.pl>

Ugh, it makes no sense that XEN won't make it into the x86_32
allmodconfig build.  Those dependencies in arch/x86/xen/Kconfig
are terrible.

For if it did, I would have caught this immediately.

[PATCH] xen: drop anti-dependency on X86_VISWS (Was: Re: [PATCH] xen: netfront: fix declaration order)

[Ian Campbell]

On Mon, 2011-04-04 at 01:24 +0100, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Sun, 03 Apr 2011 13:07:19 +0200
> 
> > [PATCH] xen: netfront: fix declaration order
> > 
> > Must declare xennet_fix_features() and xennet_set_features() before
> > using them.
> > 
> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> > Cc: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> 
> Ugh, it makes no sense that XEN won't make it into the x86_32
> allmodconfig build.  Those dependencies in arch/x86/xen/Kconfig
> are terrible.

You mean the "!X86_VISWS" I presume? It doesn't make sense to me either.
Or at least I'm not sure why this single X86_32_NON_STANDARD machine is
more special than the others to require an anti-dependency like this.

It seems to have originally appeared from f0f32fccbffa on
CONFIG_PARAVIRT due to a conflict around ARCH_SETUP() and subsequently
got pushed down to CONFIG_XEN. However ARCH_SETUP doesn't exist any more
and I think the subarch stuff has been much improved since then so there
should be no conflict any more.

I dropped the dependency and, with a bit of fiddling, was able to build
a kernel with both CONFIG_X86_VISWS and CONFIG_XEN which booted as a Xen
domU.

tglx, Andrey, to get VISWS to build I had to comment out some code in
arch/x86/platform/visws/visws_quirks.c which seems to have been missed
during some irq_chip update or something?

          CC      arch/x86/platform/visws/visws_quirks.o
        arch/x86/platform/visws/visws_quirks.c: In function 'startup_piix4_master_irq':
        arch/x86/platform/visws/visws_quirks.c:474: warning: no return statement in function returning non-void
        arch/x86/platform/visws/visws_quirks.c: At top level:
        arch/x86/platform/visws/visws_quirks.c:495: error: unknown field 'mask' specified in initializer
        arch/x86/platform/visws/visws_quirks.c:495: warning: initialization from incompatible pointer type
        arch/x86/platform/visws/visws_quirks.c: In function 'set_piix4_virtual_irq_type':
        arch/x86/platform/visws/visws_quirks.c:583: error: 'struct irq_chip' has no member named 'enable'
        arch/x86/platform/visws/visws_quirks.c:583: error: 'struct irq_chip' has no member named 'unmask'
        arch/x86/platform/visws/visws_quirks.c:584: error: 'struct irq_chip' has no member named 'disable'
        arch/x86/platform/visws/visws_quirks.c:584: error: 'struct irq_chip' has no member named 'mask'
        arch/x86/platform/visws/visws_quirks.c:585: error: 'struct irq_chip' has no member named 'unmask'
        arch/x86/platform/visws/visws_quirks.c:585: error: 'struct irq_chip' has no member named 'unmask'
        arch/x86/platform/visws/visws_quirks.c: In function 'visws_pre_intr_init':
        arch/x86/platform/visws/visws_quirks.c:602: error: expected expression before '>' token
        make[4]: *** [arch/x86/platform/visws/visws_quirks.o] Error 1

Ian

8<--------

From db0ae26f479306ee8ebcfe2a08aa56a6dfe63987 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@citrix.com>
Date: Mon, 4 Apr 2011 10:27:47 +0100
Subject: [PATCH] xen: drop anti-dependency on X86_VISWS

This seems to have been added in f0f32fccbffa to avoid a conflict arising from
the long deceased ARCH_SETUP() macro and subsequently pushed down to the XEN
option.

As far as I can tell the conflict is no longer present and by dropping the
dependency I was able to build a kernel which has both CONFIG_XEN and
CONFIG_X86_VISWS enabled and boot it on Xen. I didn't try it on the VISWS
platform.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: konrad.wilk@oracle.com
Cc: xen-devel@lists.xensource.com
Cc: Randy Dunlap <randy.dunlap@oracle.com>
Cc: Andrey Panin <pazke@donpac.ru>
Cc: linux-visws-devel@lists.sf.net
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
---
 arch/x86/xen/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
index 1c7121b..65d7b13 100644
--- a/arch/x86/xen/Kconfig
+++ b/arch/x86/xen/Kconfig
@@ -6,7 +6,7 @@ config XEN
 	bool "Xen guest support"
 	select PARAVIRT
 	select PARAVIRT_CLOCK
-	depends on X86_64 || (X86_32 && X86_PAE && !X86_VISWS)
+	depends on X86_64 || (X86_32 && X86_PAE)
 	depends on X86_CMPXCHG && X86_TSC
 	help
 	  This is the Linux Xen port.  Enabling this will allow the
-- 
1.7.2.5

Re: [PATCH] xen: drop anti-dependency on X86_VISWS

[David Miller]

From: Ian Campbell <Ian.Campbell@eu.citrix.com>
Date: Mon, 4 Apr 2011 10:55:55 +0100

> You mean the "!X86_VISWS" I presume? It doesn't make sense to me either.

No, I think 32-bit x86 allmodconfig elides XEN because of it's X86_TSC dependency.

And, well, you could type "make allmodconfig" on your tree and see for
yourself instead of asking me :-)

Re: [PATCH] xen: drop anti-dependency on X86_VISWS

[Ian Campbell]

On Wed, 2011-04-06 at 22:45 +0100, David Miller wrote:
> From: Ian Campbell <Ian.Campbell@eu.citrix.com>
> Date: Mon, 4 Apr 2011 10:55:55 +0100
> 
> > You mean the "!X86_VISWS" I presume? It doesn't make sense to me either.
> 
> No, I think 32-bit x86 allmodconfig elides XEN because of it's X86_TSC dependency.

TSC is a real dependency of the Xen interfaces.

> And, well, you could type "make allmodconfig" on your tree and see for
> yourself instead of asking me :-)

True.

X86_TSC not being enabled appears to due to CONFIG_ELAN being enabled
which causes the processor selection option (which defaults to M686,
which is a sane choice and enables TSC etc) to be gated at the top level
in arch/x86/Kconfig.cpu. Disabling the ELAN option then leaves X86_TSC
gated on !CONFIG_NUMAQ but removing that results in a generally useful
looking config.

It's a shame that these sorts of minority options cause allmodconfig to
omit support for more interesting configurations, such as modern
processors. Other than negating the semantics of such options I'm not
really sure what can be done about it though. On the other hand
compiling all the unusual stuff in an allmodconfig is probably a
positive thing.

I'm not sure why ELAN belongs in the EXTENDED_PLATFORM option space
rather than in the CPU choice option, since its only impact seems to be
on -march, MODULE_PROC_FAMILY and some cpufreq drivers which doesn't
sound like an extended platform to me but does it appear to be
deliberate (see 9e111f3e167a "x86: move ELAN to the
NON_STANDARD_PLATFORM section", that was the old name for
EXTENDED_PLATFORM).

Hrm, what about the following? (doesn't actually make a difference to
Xen since allmodconfig chooses HIGHMEM4G instead of HIGHMEM64G in the !
NUMAQ case but I stopped worrying about that several paragraphs ago)

8<--------

x86: invert X86_EXTENDED_PLATFORM to X86_STANDARD_PLATFORM

Having the =y choice be the more "standard" configuration causes
all*config to provide greater coverage of usual configurations.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index cc6c53a..6d8a404 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -299,15 +299,15 @@ config X86_BIGSMP
 	  This option is needed for the systems that have more than 8 CPUs
 
 if X86_32
-config X86_EXTENDED_PLATFORM
-	bool "Support for extended (non-PC) x86 platforms"
+config X86_STANDARD_PLATFORM
+	bool "Restrict support to standard (PC) x86 platforms"
 	default y
 	---help---
-	  If you disable this option then the kernel will only support
+	  If you enable this option then the kernel will only support
 	  standard PC platforms. (which covers the vast majority of
 	  systems out there.)
 
-	  If you enable this option then you'll be able to select support
+	  If you disable this option then you'll be able to select support
 	  for the following (non-PC) 32 bit x86 platforms:
 		AMD Elan
 		NUMAQ (IBM/Sequent)
@@ -318,25 +318,25 @@ config X86_EXTENDED_PLATFORM
 		Moorestown MID devices
 
 	  If you have one of these systems, or if you want to build a
-	  generic distribution kernel, say Y here - otherwise say N.
+	  generic distribution kernel, say N here - otherwise say Y.
 endif
 
 if X86_64
-config X86_EXTENDED_PLATFORM
-	bool "Support for extended (non-PC) x86 platforms"
+config X86_STANDARD_PLATFORM
+	bool "Restrict support to standard (PC) x86 platforms"
 	default y
 	---help---
-	  If you disable this option then the kernel will only support
+	  If you enable this option then the kernel will only support
 	  standard PC platforms. (which covers the vast majority of
 	  systems out there.)
 
-	  If you enable this option then you'll be able to select support
+	  If you disable this option then you'll be able to select support
 	  for the following (non-PC) 64 bit x86 platforms:
 		ScaleMP vSMP
 		SGI Ultraviolet
 
 	  If you have one of these systems, or if you want to build a
-	  generic distribution kernel, say Y here - otherwise say N.
+	  generic distribution kernel, say N here - otherwise say Y.
 endif
 # This is an alphabetically sorted list of 64 bit extended platforms
 # Please maintain the alphabetic order if and when there are additions
@@ -346,7 +346,7 @@ config X86_VSMP
 	select PARAVIRT_GUEST
 	select PARAVIRT
 	depends on X86_64 && PCI
-	depends on X86_EXTENDED_PLATFORM
+	depends on !X86_STANDARD_PLATFORM
 	---help---
 	  Support for ScaleMP vSMP systems.  Say 'Y' here if this kernel is
 	  supposed to run on these EM64T-based machines.  Only choose this option
@@ -355,7 +355,7 @@ config X86_VSMP
 config X86_UV
 	bool "SGI Ultraviolet"
 	depends on X86_64
-	depends on X86_EXTENDED_PLATFORM
+	depends on !X86_STANDARD_PLATFORM
 	depends on NUMA
 	depends on X86_X2APIC
 	---help---
@@ -368,7 +368,7 @@ config X86_UV
 config X86_ELAN
 	bool "AMD Elan"
 	depends on X86_32
-	depends on X86_EXTENDED_PLATFORM
+	depends on !X86_STANDARD_PLATFORM
 	---help---
 	  Select this for an AMD Elan processor.
 
@@ -381,7 +381,7 @@ config X86_INTEL_CE
 	depends on PCI
 	depends on PCI_GODIRECT
 	depends on X86_32
-	depends on X86_EXTENDED_PLATFORM
+	depends on !X86_STANDARD_PLATFORM
 	select X86_REBOOTFIXUPS
 	select OF
 	select OF_EARLY_FLATTREE
@@ -395,7 +395,7 @@ config X86_MRST
 	depends on PCI
 	depends on PCI_GOANY
 	depends on X86_32
-	depends on X86_EXTENDED_PLATFORM
+	depends on !X86_STANDARD_PLATFORM
 	depends on X86_IO_APIC
 	select APB_TIMER
 	select I2C
@@ -413,7 +413,7 @@ config X86_MRST
 config X86_RDC321X
 	bool "RDC R-321x SoC"
 	depends on X86_32
-	depends on X86_EXTENDED_PLATFORM
+	depends on !X86_STANDARD_PLATFORM
 	select M486
 	select X86_REBOOTFIXUPS
 	---help---
@@ -424,7 +424,7 @@ config X86_RDC321X
 config X86_32_NON_STANDARD
 	bool "Support non-standard 32-bit SMP architectures"
 	depends on X86_32 && SMP
-	depends on X86_EXTENDED_PLATFORM
+	depends on !X86_STANDARD_PLATFORM
 	---help---
 	  This option compiles in the NUMAQ, Summit, bigsmp, ES7000, default
 	  subarchitectures.  It is intended for a generic binary kernel.

Re: [PATCH] xen: drop anti-dependency on X86_VISWS

[H. Peter Anvin]

On 04/06/2011 11:58 PM, Ian Campbell wrote:
> 
> I'm not sure why ELAN belongs in the EXTENDED_PLATFORM option space
> rather than in the CPU choice option, since its only impact seems to be
> on -march, MODULE_PROC_FAMILY and some cpufreq drivers which doesn't
> sound like an extended platform to me but does it appear to be
> deliberate (see 9e111f3e167a "x86: move ELAN to the
> NON_STANDARD_PLATFORM section", that was the old name for
> EXTENDED_PLATFORM).
> 

Historic... we used to have nonstandard A20M# handling on Elan, until it
was discovered that we could make it work without it.

	-hpa


-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] xen: drop anti-dependency on X86_VISWS

[Ian Campbell]

(dropping netdev and visws list)
On Thu, 2011-04-07 at 18:00 +0100, H. Peter Anvin wrote:
> On 04/06/2011 11:58 PM, Ian Campbell wrote:
> > 
> > I'm not sure why ELAN belongs in the EXTENDED_PLATFORM option space
> > rather than in the CPU choice option, since its only impact seems to be
> > on -march, MODULE_PROC_FAMILY and some cpufreq drivers which doesn't
> > sound like an extended platform to me but does it appear to be
> > deliberate (see 9e111f3e167a "x86: move ELAN to the
> > NON_STANDARD_PLATFORM section", that was the old name for
> > EXTENDED_PLATFORM).
> > 
> 
> Historic... we used to have nonstandard A20M# handling on Elan, until it
> was discovered that we could make it work without it.

Any reason not switch it over at this point then?

8<--------------------------

From b1942fa168aee77537bf467e4c68c6f181b8fdee Mon Sep 17 00:00:00 2001
From: Ian Campbell <ijc@hellion.org.uk>
Date: Fri, 8 Apr 2011 07:42:29 +0100
Subject: [PATCH] x86: move AMD Elan Kconfig under "Processor family"

Currently the option resides under X86_EXTENDED_PLATFORM due to historical
nonstandard A20M# handling. However that is no longer the case and so Elan can
be treated as part of the standard processor choice Kconfig option.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/Kconfig                    |   11 -----------
 arch/x86/Kconfig.cpu                |   16 ++++++++++------
 arch/x86/Makefile_32.cpu            |    2 +-
 arch/x86/include/asm/module.h       |    2 +-
 arch/x86/kernel/cpu/cpufreq/Kconfig |    4 ++--
 5 files changed, 14 insertions(+), 21 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index cc6c53a..f00a3f3 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -365,17 +365,6 @@ config X86_UV
 # Following is an alphabetically sorted list of 32 bit extended platforms
 # Please maintain the alphabetic order if and when there are additions
 
-config X86_ELAN
-	bool "AMD Elan"
-	depends on X86_32
-	depends on X86_EXTENDED_PLATFORM
-	---help---
-	  Select this for an AMD Elan processor.
-
-	  Do not use this option for K6/Athlon/Opteron processors!
-
-	  If unsure, choose "PC-compatible" instead.
-
 config X86_INTEL_CE
 	bool "CE4100 TV platform"
 	depends on PCI
diff --git a/arch/x86/Kconfig.cpu b/arch/x86/Kconfig.cpu
index d161e93..6a7cfdf 100644
--- a/arch/x86/Kconfig.cpu
+++ b/arch/x86/Kconfig.cpu
@@ -1,6 +1,4 @@
 # Put here option for CPU selection and depending optimization
-if !X86_ELAN
-
 choice
 	prompt "Processor family"
 	default M686 if X86_32
@@ -203,6 +201,14 @@ config MWINCHIP3D
 	  stores for this CPU, which can increase performance of some
 	  operations.
 
+config MELAN
+	bool "AMD Elan"
+	depends on X86_32
+	---help---
+	  Select this for an AMD Elan processor.
+
+	  Do not use this option for K6/Athlon/Opteron processors!
+
 config MGEODEGX1
 	bool "GeodeGX1"
 	depends on X86_32
@@ -292,8 +298,6 @@ config X86_GENERIC
 	  This is really intended for distributors who need more
 	  generic optimizations.
 
-endif
-
 #
 # Define implied options from the CPU selection here
 config X86_INTERNODE_CACHE_SHIFT
@@ -312,7 +316,7 @@ config X86_L1_CACHE_SHIFT
 	int
 	default "7" if MPENTIUM4 || MPSC
 	default "6" if MK7 || MK8 || MPENTIUMM || MCORE2 || MATOM || MVIAC7 || X86_GENERIC || GENERIC_CPU
-	default "4" if X86_ELAN || M486 || M386 || MGEODEGX1
+	default "4" if MELAN || M486 || M386 || MGEODEGX1
 	default "5" if MWINCHIP3D || MWINCHIPC6 || MCRUSOE || MEFFICEON || MCYRIXIII || MK6 || MPENTIUMIII || MPENTIUMII || M686 || M586MMX || M586TSC || M586 || MVIAC3_2 || MGEODE_LX
 
 config X86_XADD
@@ -358,7 +362,7 @@ config X86_POPAD_OK
 
 config X86_ALIGNMENT_16
 	def_bool y
-	depends on MWINCHIP3D || MWINCHIPC6 || MCYRIXIII || X86_ELAN || MK6 || M586MMX || M586TSC || M586 || M486 || MVIAC3_2 || MGEODEGX1
+	depends on MWINCHIP3D || MWINCHIPC6 || MCYRIXIII || MELAN || MK6 || M586MMX || M586TSC || M586 || M486 || MVIAC3_2 || MGEODEGX1
 
 config X86_INTEL_USERCOPY
 	def_bool y
diff --git a/arch/x86/Makefile_32.cpu b/arch/x86/Makefile_32.cpu
index f2ee1ab..86cee7b 100644
--- a/arch/x86/Makefile_32.cpu
+++ b/arch/x86/Makefile_32.cpu
@@ -37,7 +37,7 @@ cflags-$(CONFIG_MATOM)		+= $(call cc-option,-march=atom,$(call cc-option,-march=
 	$(call cc-option,-mtune=atom,$(call cc-option,-mtune=generic))
 
 # AMD Elan support
-cflags-$(CONFIG_X86_ELAN)	+= -march=i486
+cflags-$(CONFIG_MELAN)		+= -march=i486
 
 # Geode GX1 support
 cflags-$(CONFIG_MGEODEGX1)	+= -march=pentium-mmx
diff --git a/arch/x86/include/asm/module.h b/arch/x86/include/asm/module.h
index 67763c5..9eae775 100644
--- a/arch/x86/include/asm/module.h
+++ b/arch/x86/include/asm/module.h
@@ -35,7 +35,7 @@
 #define MODULE_PROC_FAMILY "K7 "
 #elif defined CONFIG_MK8
 #define MODULE_PROC_FAMILY "K8 "
-#elif defined CONFIG_X86_ELAN
+#elif defined CONFIG_MELAN
 #define MODULE_PROC_FAMILY "ELAN "
 #elif defined CONFIG_MCRUSOE
 #define MODULE_PROC_FAMILY "CRUSOE "
diff --git a/arch/x86/kernel/cpu/cpufreq/Kconfig b/arch/x86/kernel/cpu/cpufreq/Kconfig
index 870e6cc..0ab9b22 100644
--- a/arch/x86/kernel/cpu/cpufreq/Kconfig
+++ b/arch/x86/kernel/cpu/cpufreq/Kconfig
@@ -43,7 +43,7 @@ config X86_ACPI_CPUFREQ
 config ELAN_CPUFREQ
 	tristate "AMD Elan SC400 and SC410"
 	select CPU_FREQ_TABLE
-	depends on X86_ELAN
+	depends on MELAN
 	---help---
 	  This adds the CPUFreq driver for AMD Elan SC400 and SC410
 	  processors.
@@ -59,7 +59,7 @@ config ELAN_CPUFREQ
 config SC520_CPUFREQ
 	tristate "AMD Elan SC520"
 	select CPU_FREQ_TABLE
-	depends on X86_ELAN
+	depends on MELAN
 	---help---
 	  This adds the CPUFreq driver for AMD Elan SC520 processor.
 
-- 
1.7.4.1

Re: [PATCH] xen: drop anti-dependency on X86_VISWS

[Ian Campbell]

(dropping netdev and visws list)
On Thu, 2011-04-07 at 18:00 +0100, H. Peter Anvin wrote:
> On 04/06/2011 11:58 PM, Ian Campbell wrote:
> > 
> > I'm not sure why ELAN belongs in the EXTENDED_PLATFORM option space
> > rather than in the CPU choice option, since its only impact seems to be
> > on -march, MODULE_PROC_FAMILY and some cpufreq drivers which doesn't
> > sound like an extended platform to me but does it appear to be
> > deliberate (see 9e111f3e167a "x86: move ELAN to the
> > NON_STANDARD_PLATFORM section", that was the old name for
> > EXTENDED_PLATFORM).
> > 
> 
> Historic... we used to have nonstandard A20M# handling on Elan, until it
> was discovered that we could make it work without it.

Any reason not switch it over at this point then?

8<--------------------------

>From b1942fa168aee77537bf467e4c68c6f181b8fdee Mon Sep 17 00:00:00 2001
From: Ian Campbell <ijc@hellion.org.uk>
Date: Fri, 8 Apr 2011 07:42:29 +0100
Subject: [PATCH] x86: move AMD Elan Kconfig under "Processor family"

Currently the option resides under X86_EXTENDED_PLATFORM due to historical
nonstandard A20M# handling. However that is no longer the case and so Elan can
be treated as part of the standard processor choice Kconfig option.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/Kconfig                    |   11 -----------
 arch/x86/Kconfig.cpu                |   16 ++++++++++------
 arch/x86/Makefile_32.cpu            |    2 +-
 arch/x86/include/asm/module.h       |    2 +-
 arch/x86/kernel/cpu/cpufreq/Kconfig |    4 ++--
 5 files changed, 14 insertions(+), 21 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index cc6c53a..f00a3f3 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -365,17 +365,6 @@ config X86_UV
 # Following is an alphabetically sorted list of 32 bit extended platforms
 # Please maintain the alphabetic order if and when there are additions
 
-config X86_ELAN
-	bool "AMD Elan"
-	depends on X86_32
-	depends on X86_EXTENDED_PLATFORM
-	---help---
-	  Select this for an AMD Elan processor.
-
-	  Do not use this option for K6/Athlon/Opteron processors!
-
-	  If unsure, choose "PC-compatible" instead.
-
 config X86_INTEL_CE
 	bool "CE4100 TV platform"
 	depends on PCI
diff --git a/arch/x86/Kconfig.cpu b/arch/x86/Kconfig.cpu
index d161e93..6a7cfdf 100644
--- a/arch/x86/Kconfig.cpu
+++ b/arch/x86/Kconfig.cpu
@@ -1,6 +1,4 @@
 # Put here option for CPU selection and depending optimization
-if !X86_ELAN
-
 choice
 	prompt "Processor family"
 	default M686 if X86_32
@@ -203,6 +201,14 @@ config MWINCHIP3D
 	  stores for this CPU, which can increase performance of some
 	  operations.
 
+config MELAN
+	bool "AMD Elan"
+	depends on X86_32
+	---help---
+	  Select this for an AMD Elan processor.
+
+	  Do not use this option for K6/Athlon/Opteron processors!
+
 config MGEODEGX1
 	bool "GeodeGX1"
 	depends on X86_32
@@ -292,8 +298,6 @@ config X86_GENERIC
 	  This is really intended for distributors who need more
 	  generic optimizations.
 
-endif
-
 #
 # Define implied options from the CPU selection here
 config X86_INTERNODE_CACHE_SHIFT
@@ -312,7 +316,7 @@ config X86_L1_CACHE_SHIFT
 	int
 	default "7" if MPENTIUM4 || MPSC
 	default "6" if MK7 || MK8 || MPENTIUMM || MCORE2 || MATOM || MVIAC7 || X86_GENERIC || GENERIC_CPU
-	default "4" if X86_ELAN || M486 || M386 || MGEODEGX1
+	default "4" if MELAN || M486 || M386 || MGEODEGX1
 	default "5" if MWINCHIP3D || MWINCHIPC6 || MCRUSOE || MEFFICEON || MCYRIXIII || MK6 || MPENTIUMIII || MPENTIUMII || M686 || M586MMX || M586TSC || M586 || MVIAC3_2 || MGEODE_LX
 
 config X86_XADD
@@ -358,7 +362,7 @@ config X86_POPAD_OK
 
 config X86_ALIGNMENT_16
 	def_bool y
-	depends on MWINCHIP3D || MWINCHIPC6 || MCYRIXIII || X86_ELAN || MK6 || M586MMX || M586TSC || M586 || M486 || MVIAC3_2 || MGEODEGX1
+	depends on MWINCHIP3D || MWINCHIPC6 || MCYRIXIII || MELAN || MK6 || M586MMX || M586TSC || M586 || M486 || MVIAC3_2 || MGEODEGX1
 
 config X86_INTEL_USERCOPY
 	def_bool y
diff --git a/arch/x86/Makefile_32.cpu b/arch/x86/Makefile_32.cpu
index f2ee1ab..86cee7b 100644
--- a/arch/x86/Makefile_32.cpu
+++ b/arch/x86/Makefile_32.cpu
@@ -37,7 +37,7 @@ cflags-$(CONFIG_MATOM)		+= $(call cc-option,-march=atom,$(call cc-option,-march=
 	$(call cc-option,-mtune=atom,$(call cc-option,-mtune=generic))
 
 # AMD Elan support
-cflags-$(CONFIG_X86_ELAN)	+= -march=i486
+cflags-$(CONFIG_MELAN)		+= -march=i486
 
 # Geode GX1 support
 cflags-$(CONFIG_MGEODEGX1)	+= -march=pentium-mmx
diff --git a/arch/x86/include/asm/module.h b/arch/x86/include/asm/module.h
index 67763c5..9eae775 100644
--- a/arch/x86/include/asm/module.h
+++ b/arch/x86/include/asm/module.h
@@ -35,7 +35,7 @@
 #define MODULE_PROC_FAMILY "K7 "
 #elif defined CONFIG_MK8
 #define MODULE_PROC_FAMILY "K8 "
-#elif defined CONFIG_X86_ELAN
+#elif defined CONFIG_MELAN
 #define MODULE_PROC_FAMILY "ELAN "
 #elif defined CONFIG_MCRUSOE
 #define MODULE_PROC_FAMILY "CRUSOE "
diff --git a/arch/x86/kernel/cpu/cpufreq/Kconfig b/arch/x86/kernel/cpu/cpufreq/Kconfig
index 870e6cc..0ab9b22 100644
--- a/arch/x86/kernel/cpu/cpufreq/Kconfig
+++ b/arch/x86/kernel/cpu/cpufreq/Kconfig
@@ -43,7 +43,7 @@ config X86_ACPI_CPUFREQ
 config ELAN_CPUFREQ
 	tristate "AMD Elan SC400 and SC410"
 	select CPU_FREQ_TABLE
-	depends on X86_ELAN
+	depends on MELAN
 	---help---
 	  This adds the CPUFreq driver for AMD Elan SC400 and SC410
 	  processors.
@@ -59,7 +59,7 @@ config ELAN_CPUFREQ
 config SC520_CPUFREQ
 	tristate "AMD Elan SC520"
 	select CPU_FREQ_TABLE
-	depends on X86_ELAN
+	depends on MELAN
 	---help---
 	  This adds the CPUFreq driver for AMD Elan SC520 processor.
 
-- 
1.7.4.1

Re: [PATCH] xen: drop anti-dependency on X86_VISWS

[H. Peter Anvin]

On 04/07/2011 11:46 PM, Ian Campbell wrote:
> 
> Any reason not switch it over at this point then?
> 

Not really.

	-hpa

Re: [PATCH] xen: drop anti-dependency on X86_VISWS

[H. Peter Anvin]

On 04/07/2011 11:46 PM, Ian Campbell wrote:
> 
> Any reason not switch it over at this point then?
> 

Not really.

	-hpa

[tip:x86/cpu] x86, cpu: Move AMD Elan Kconfig under "Processor family"

[tip-bot for Ian Campbell]

Commit-ID:  ce9c99af8d4b3b0b9463654fd252d8640d804dc3
Gitweb:     http://git.kernel.org/tip/ce9c99af8d4b3b0b9463654fd252d8640d804dc3
Author:     Ian Campbell <ijc@hellion.org.uk>
AuthorDate: Fri, 8 Apr 2011 07:42:29 +0100
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Fri, 8 Apr 2011 13:01:25 -0700

x86, cpu: Move AMD Elan Kconfig under "Processor family"

Currently the option resides under X86_EXTENDED_PLATFORM due to historical
nonstandard A20M# handling. However that is no longer the case and so Elan can
be treated as part of the standard processor choice Kconfig option.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Link: http://lkml.kernel.org/r/1302245177.31620.47.camel@localhost.localdomain
Cc: H. Peter Anvin <hpa@zytor.com>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/Kconfig                    |   11 -----------
 arch/x86/Kconfig.cpu                |   16 ++++++++++------
 arch/x86/Makefile_32.cpu            |    2 +-
 arch/x86/include/asm/module.h       |    2 +-
 arch/x86/kernel/cpu/cpufreq/Kconfig |    4 ++--
 5 files changed, 14 insertions(+), 21 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index cc6c53a..f00a3f3 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -365,17 +365,6 @@ config X86_UV
 # Following is an alphabetically sorted list of 32 bit extended platforms
 # Please maintain the alphabetic order if and when there are additions
 
-config X86_ELAN
-	bool "AMD Elan"
-	depends on X86_32
-	depends on X86_EXTENDED_PLATFORM
-	---help---
-	  Select this for an AMD Elan processor.
-
-	  Do not use this option for K6/Athlon/Opteron processors!
-
-	  If unsure, choose "PC-compatible" instead.
-
 config X86_INTEL_CE
 	bool "CE4100 TV platform"
 	depends on PCI
diff --git a/arch/x86/Kconfig.cpu b/arch/x86/Kconfig.cpu
index d161e93..6a7cfdf 100644
--- a/arch/x86/Kconfig.cpu
+++ b/arch/x86/Kconfig.cpu
@@ -1,6 +1,4 @@
 # Put here option for CPU selection and depending optimization
-if !X86_ELAN
-
 choice
 	prompt "Processor family"
 	default M686 if X86_32
@@ -203,6 +201,14 @@ config MWINCHIP3D
 	  stores for this CPU, which can increase performance of some
 	  operations.
 
+config MELAN
+	bool "AMD Elan"
+	depends on X86_32
+	---help---
+	  Select this for an AMD Elan processor.
+
+	  Do not use this option for K6/Athlon/Opteron processors!
+
 config MGEODEGX1
 	bool "GeodeGX1"
 	depends on X86_32
@@ -292,8 +298,6 @@ config X86_GENERIC
 	  This is really intended for distributors who need more
 	  generic optimizations.
 
-endif
-
 #
 # Define implied options from the CPU selection here
 config X86_INTERNODE_CACHE_SHIFT
@@ -312,7 +316,7 @@ config X86_L1_CACHE_SHIFT
 	int
 	default "7" if MPENTIUM4 || MPSC
 	default "6" if MK7 || MK8 || MPENTIUMM || MCORE2 || MATOM || MVIAC7 || X86_GENERIC || GENERIC_CPU
-	default "4" if X86_ELAN || M486 || M386 || MGEODEGX1
+	default "4" if MELAN || M486 || M386 || MGEODEGX1
 	default "5" if MWINCHIP3D || MWINCHIPC6 || MCRUSOE || MEFFICEON || MCYRIXIII || MK6 || MPENTIUMIII || MPENTIUMII || M686 || M586MMX || M586TSC || M586 || MVIAC3_2 || MGEODE_LX
 
 config X86_XADD
@@ -358,7 +362,7 @@ config X86_POPAD_OK
 
 config X86_ALIGNMENT_16
 	def_bool y
-	depends on MWINCHIP3D || MWINCHIPC6 || MCYRIXIII || X86_ELAN || MK6 || M586MMX || M586TSC || M586 || M486 || MVIAC3_2 || MGEODEGX1
+	depends on MWINCHIP3D || MWINCHIPC6 || MCYRIXIII || MELAN || MK6 || M586MMX || M586TSC || M586 || M486 || MVIAC3_2 || MGEODEGX1
 
 config X86_INTEL_USERCOPY
 	def_bool y
diff --git a/arch/x86/Makefile_32.cpu b/arch/x86/Makefile_32.cpu
index f2ee1ab..86cee7b 100644
--- a/arch/x86/Makefile_32.cpu
+++ b/arch/x86/Makefile_32.cpu
@@ -37,7 +37,7 @@ cflags-$(CONFIG_MATOM)		+= $(call cc-option,-march=atom,$(call cc-option,-march=
 	$(call cc-option,-mtune=atom,$(call cc-option,-mtune=generic))
 
 # AMD Elan support
-cflags-$(CONFIG_X86_ELAN)	+= -march=i486
+cflags-$(CONFIG_MELAN)		+= -march=i486
 
 # Geode GX1 support
 cflags-$(CONFIG_MGEODEGX1)	+= -march=pentium-mmx
diff --git a/arch/x86/include/asm/module.h b/arch/x86/include/asm/module.h
index 67763c5..9eae775 100644
--- a/arch/x86/include/asm/module.h
+++ b/arch/x86/include/asm/module.h
@@ -35,7 +35,7 @@
 #define MODULE_PROC_FAMILY "K7 "
 #elif defined CONFIG_MK8
 #define MODULE_PROC_FAMILY "K8 "
-#elif defined CONFIG_X86_ELAN
+#elif defined CONFIG_MELAN
 #define MODULE_PROC_FAMILY "ELAN "
 #elif defined CONFIG_MCRUSOE
 #define MODULE_PROC_FAMILY "CRUSOE "
diff --git a/arch/x86/kernel/cpu/cpufreq/Kconfig b/arch/x86/kernel/cpu/cpufreq/Kconfig
index 870e6cc..0ab9b22 100644
--- a/arch/x86/kernel/cpu/cpufreq/Kconfig
+++ b/arch/x86/kernel/cpu/cpufreq/Kconfig
@@ -43,7 +43,7 @@ config X86_ACPI_CPUFREQ
 config ELAN_CPUFREQ
 	tristate "AMD Elan SC400 and SC410"
 	select CPU_FREQ_TABLE
-	depends on X86_ELAN
+	depends on MELAN
 	---help---
 	  This adds the CPUFreq driver for AMD Elan SC400 and SC410
 	  processors.
@@ -59,7 +59,7 @@ config ELAN_CPUFREQ
 config SC520_CPUFREQ
 	tristate "AMD Elan SC520"
 	select CPU_FREQ_TABLE
-	depends on X86_ELAN
+	depends on MELAN
 	---help---
 	  This adds the CPUFreq driver for AMD Elan SC520 processor.
 

Re: [PATCH] xen: drop anti-dependency on X86_VISWS

[H. Peter Anvin]

On 04/06/2011 11:58 PM, Ian Campbell wrote:
> 
> I'm not sure why ELAN belongs in the EXTENDED_PLATFORM option space
> rather than in the CPU choice option, since its only impact seems to be
> on -march, MODULE_PROC_FAMILY and some cpufreq drivers which doesn't
> sound like an extended platform to me but does it appear to be
> deliberate (see 9e111f3e167a "x86: move ELAN to the
> NON_STANDARD_PLATFORM section", that was the old name for
> EXTENDED_PLATFORM).
> 

Historic... we used to have nonstandard A20M# handling on Elan, until it
was discovered that we could make it work without it.

	-hpa


-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] xen: drop anti-dependency on X86_VISWS

[Jeremy Fitzhardinge]

On 04/06/2011 11:58 PM, Ian Campbell wrote:
> On Wed, 2011-04-06 at 22:45 +0100, David Miller wrote:
>> From: Ian Campbell <Ian.Campbell@eu.citrix.com>
>> Date: Mon, 4 Apr 2011 10:55:55 +0100
>>
>>> You mean the "!X86_VISWS" I presume? It doesn't make sense to me either.
>> No, I think 32-bit x86 allmodconfig elides XEN because of it's X86_TSC dependency.
> TSC is a real dependency of the Xen interfaces.

Not really.  The TSC register is a requirement, but that's going to be
present on any CPU which can boot Xen.  We don't need any of the
kernel's TSC machinery though.

    J

Re: [PATCH] xen: drop anti-dependency on X86_VISWS

[Ian Campbell]

(dropping netdev and the visws list)

On Thu, 2011-04-07 at 11:07 -0700, Jeremy Fitzhardinge wrote:
> On 04/06/2011 11:58 PM, Ian Campbell wrote:
> > On Wed, 2011-04-06 at 22:45 +0100, David Miller wrote:
> >> From: Ian Campbell <Ian.Campbell@eu.citrix.com>
> >> Date: Mon, 4 Apr 2011 10:55:55 +0100
> >>
> >>> You mean the "!X86_VISWS" I presume? It doesn't make sense to me either.
> >> No, I think 32-bit x86 allmodconfig elides XEN because of it's X86_TSC dependency.
> > TSC is a real dependency of the Xen interfaces.
> 
> Not really.  The TSC register is a requirement, but that's going to be
> present on any CPU which can boot Xen.  We don't need any of the
> kernel's TSC machinery though.

So why the Kconfig dependency then? In principal a kernel compiled for a
non-TSC processor (which meets the other requirements for Xen, such as
PAE support) will run just fine under Xen on a newer piece of hardware.

Is there any downside to this patch (is X86_CMPXCHG in the same sort of
boat?)

8<----------------------------------

From 7204945696a927d281366f2a57baee37e2b43ca3 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ijc@hellion.org.uk>
Date: Fri, 8 Apr 2011 07:33:21 +0100
Subject: [PATCH] xen: remove Kconfig dependency on X86_TSC

The TSC register is a requirement when running under Xen, but that's going to
be present on any CPU which can boot Xen. We don't need any of the kernel's TSC
machinery, since the usage is contained within the Xen interfaces, and therefore
XEN does not need to depend on CONFIG_X86_TSC.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 arch/x86/xen/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
index 1c7121b..ac69c5b 100644
--- a/arch/x86/xen/Kconfig
+++ b/arch/x86/xen/Kconfig
@@ -7,7 +7,7 @@ config XEN
 	select PARAVIRT
 	select PARAVIRT_CLOCK
 	depends on X86_64 || (X86_32 && X86_PAE && !X86_VISWS)
-	depends on X86_CMPXCHG && X86_TSC
+	depends on X86_CMPXCHG
 	help
 	  This is the Linux Xen port.  Enabling this will allow the
 	  kernel to boot in a paravirtualized environment under the
-- 
1.7.4.1

Re: [PATCH] xen: drop anti-dependency on X86_VISWS

[Ian Campbell]

(dropping netdev and the visws list)

On Thu, 2011-04-07 at 11:07 -0700, Jeremy Fitzhardinge wrote:
> On 04/06/2011 11:58 PM, Ian Campbell wrote:
> > On Wed, 2011-04-06 at 22:45 +0100, David Miller wrote:
> >> From: Ian Campbell <Ian.Campbell@eu.citrix.com>
> >> Date: Mon, 4 Apr 2011 10:55:55 +0100
> >>
> >>> You mean the "!X86_VISWS" I presume? It doesn't make sense to me either.
> >> No, I think 32-bit x86 allmodconfig elides XEN because of it's X86_TSC dependency.
> > TSC is a real dependency of the Xen interfaces.
> 
> Not really.  The TSC register is a requirement, but that's going to be
> present on any CPU which can boot Xen.  We don't need any of the
> kernel's TSC machinery though.

So why the Kconfig dependency then? In principal a kernel compiled for a
non-TSC processor (which meets the other requirements for Xen, such as
PAE support) will run just fine under Xen on a newer piece of hardware.

Is there any downside to this patch (is X86_CMPXCHG in the same sort of
boat?)

8<----------------------------------

>From 7204945696a927d281366f2a57baee37e2b43ca3 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ijc@hellion.org.uk>
Date: Fri, 8 Apr 2011 07:33:21 +0100
Subject: [PATCH] xen: remove Kconfig dependency on X86_TSC

The TSC register is a requirement when running under Xen, but that's going to
be present on any CPU which can boot Xen. We don't need any of the kernel's TSC
machinery, since the usage is contained within the Xen interfaces, and therefore
XEN does not need to depend on CONFIG_X86_TSC.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 arch/x86/xen/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
index 1c7121b..ac69c5b 100644
--- a/arch/x86/xen/Kconfig
+++ b/arch/x86/xen/Kconfig
@@ -7,7 +7,7 @@ config XEN
 	select PARAVIRT
 	select PARAVIRT_CLOCK
 	depends on X86_64 || (X86_32 && X86_PAE && !X86_VISWS)
-	depends on X86_CMPXCHG && X86_TSC
+	depends on X86_CMPXCHG
 	help
 	  This is the Linux Xen port.  Enabling this will allow the
 	  kernel to boot in a paravirtualized environment under the
-- 
1.7.4.1

Re: [PATCH] xen: drop anti-dependency on X86_VISWS

[Jeremy Fitzhardinge]

On 04/07/2011 11:38 PM, Ian Campbell wrote:
>> Not really.  The TSC register is a requirement, but that's going to be
>> present on any CPU which can boot Xen.  We don't need any of the
>> kernel's TSC machinery though.
> So why the Kconfig dependency then? In principal a kernel compiled for a
> non-TSC processor (which meets the other requirements for Xen, such as
> PAE support) will run just fine under Xen on a newer piece of hardware.

Not sure where it came from.  It was probably never needed, or just
added for some secondary effect we wanted.

> Is there any downside to this patch (is X86_CMPXCHG in the same sort of
> boat?)

Only if we don't use cmpxchg in shared memory with other domains or the
hypervisor.  (I don't think it will dynamically switch between real and
emulated cmpxchg depending on availability.)

    J

[Xen-devel] Re: [PATCH] xen: drop anti-dependency on X86_VISWS

[Jan Beulich]

>>> On 08.04.11 at 17:25, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> On 04/07/2011 11:38 PM, Ian Campbell wrote:
>> Is there any downside to this patch (is X86_CMPXCHG in the same sort of
>> boat?)
> 
> Only if we don't use cmpxchg in shared memory with other domains or the
> hypervisor.  (I don't think it will dynamically switch between real and
> emulated cmpxchg depending on availability.)

Actually it does - see the "#ifndef CONFIG_X86_CMPXCHG" section
in asm/cmpxchg_32.h.

Jan

Re: [PATCH] xen: drop anti-dependency on X86_VISWS

[Jan Beulich]

>>> On 08.04.11 at 17:25, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> On 04/07/2011 11:38 PM, Ian Campbell wrote:
>> Is there any downside to this patch (is X86_CMPXCHG in the same sort of
>> boat?)
> 
> Only if we don't use cmpxchg in shared memory with other domains or the
> hypervisor.  (I don't think it will dynamically switch between real and
> emulated cmpxchg depending on availability.)

Actually it does - see the "#ifndef CONFIG_X86_CMPXCHG" section
in asm/cmpxchg_32.h.

Jan

Re: Re: [PATCH] xen: drop anti-dependency on X86_VISWS

[Jeremy Fitzhardinge]

On 04/08/2011 08:42 AM, Jan Beulich wrote:
>>>> On 08.04.11 at 17:25, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
>> On 04/07/2011 11:38 PM, Ian Campbell wrote:
>>> Is there any downside to this patch (is X86_CMPXCHG in the same sort of
>>> boat?)
>> Only if we don't use cmpxchg in shared memory with other domains or the
>> hypervisor.  (I don't think it will dynamically switch between real and
>> emulated cmpxchg depending on availability.)
> Actually it does - see the "#ifndef CONFIG_X86_CMPXCHG" section
> in asm/cmpxchg_32.h.

Hm, OK.  Still, I'm happiest with that dependency in case someone
knobbles the cpu to exclude cmpxchg and breaks things.

    J

Re: Re: [PATCH] xen: drop anti-dependency on X86_VISWS

[Ian Campbell]

On Fri, 2011-04-08 at 11:24 -0700, Jeremy Fitzhardinge wrote:
> On 04/08/2011 08:42 AM, Jan Beulich wrote:
> >>>> On 08.04.11 at 17:25, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> >> On 04/07/2011 11:38 PM, Ian Campbell wrote:
> >>> Is there any downside to this patch (is X86_CMPXCHG in the same sort of
> >>> boat?)
> >> Only if we don't use cmpxchg in shared memory with other domains or the
> >> hypervisor.  (I don't think it will dynamically switch between real and
> >> emulated cmpxchg depending on availability.)

We do use cmpxchg in the grant table code at least (actually,
sync_cmpxchng in that case).

> > Actually it does - see the "#ifndef CONFIG_X86_CMPXCHG" section
> > in asm/cmpxchg_32.h.
> 
> Hm, OK.  Still, I'm happiest with that dependency in case someone
> knobbles the cpu to exclude cmpxchg and breaks things.

Dropping the TSC patch is sensible though?

Ian.

Re: Re: [PATCH] xen: drop anti-dependency on X86_VISWS

[Jeremy Fitzhardinge]

On 04/09/2011 03:34 AM, Ian Campbell wrote:
>>> Actually it does - see the "#ifndef CONFIG_X86_CMPXCHG" section
>>> in asm/cmpxchg_32.h.
>> Hm, OK.  Still, I'm happiest with that dependency in case someone
>> knobbles the cpu to exclude cmpxchg and breaks things.
> Dropping the TSC patch is sensible though?

You mean dropping the TSC dependency?  Yes, I think so.

    J

Re: [Xen-devel] Re: [PATCH] xen: drop anti-dependency on X86_VISWS

[Jeremy Fitzhardinge]

On 04/09/2011 03:34 AM, Ian Campbell wrote:
>>> Actually it does - see the "#ifndef CONFIG_X86_CMPXCHG" section
>>> in asm/cmpxchg_32.h.
>> Hm, OK.  Still, I'm happiest with that dependency in case someone
>> knobbles the cpu to exclude cmpxchg and breaks things.
> Dropping the TSC patch is sensible though?

You mean dropping the TSC dependency?  Yes, I think so.

    J

Re: [Xen-devel] Re: [PATCH] xen: drop anti-dependency on X86_VISWS

[Ian Campbell]

On Fri, 2011-04-08 at 11:24 -0700, Jeremy Fitzhardinge wrote:
> On 04/08/2011 08:42 AM, Jan Beulich wrote:
> >>>> On 08.04.11 at 17:25, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> >> On 04/07/2011 11:38 PM, Ian Campbell wrote:
> >>> Is there any downside to this patch (is X86_CMPXCHG in the same sort of
> >>> boat?)
> >> Only if we don't use cmpxchg in shared memory with other domains or the
> >> hypervisor.  (I don't think it will dynamically switch between real and
> >> emulated cmpxchg depending on availability.)

We do use cmpxchg in the grant table code at least (actually,
sync_cmpxchng in that case).

> > Actually it does - see the "#ifndef CONFIG_X86_CMPXCHG" section
> > in asm/cmpxchg_32.h.
> 
> Hm, OK.  Still, I'm happiest with that dependency in case someone
> knobbles the cpu to exclude cmpxchg and breaks things.

Dropping the TSC patch is sensible though?

Ian.

Re: [Xen-devel] Re: [PATCH] xen: drop anti-dependency on X86_VISWS

[Jeremy Fitzhardinge]

On 04/08/2011 08:42 AM, Jan Beulich wrote:
>>>> On 08.04.11 at 17:25, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
>> On 04/07/2011 11:38 PM, Ian Campbell wrote:
>>> Is there any downside to this patch (is X86_CMPXCHG in the same sort of
>>> boat?)
>> Only if we don't use cmpxchg in shared memory with other domains or the
>> hypervisor.  (I don't think it will dynamically switch between real and
>> emulated cmpxchg depending on availability.)
> Actually it does - see the "#ifndef CONFIG_X86_CMPXCHG" section
> in asm/cmpxchg_32.h.

Hm, OK.  Still, I'm happiest with that dependency in case someone
knobbles the cpu to exclude cmpxchg and breaks things.

    J

Re: [PATCH] xen: drop anti-dependency on X86_VISWS

[Jeremy Fitzhardinge]

On 04/07/2011 11:38 PM, Ian Campbell wrote:
>> Not really.  The TSC register is a requirement, but that's going to be
>> present on any CPU which can boot Xen.  We don't need any of the
>> kernel's TSC machinery though.
> So why the Kconfig dependency then? In principal a kernel compiled for a
> non-TSC processor (which meets the other requirements for Xen, such as
> PAE support) will run just fine under Xen on a newer piece of hardware.

Not sure where it came from.  It was probably never needed, or just
added for some secondary effect we wanted.

> Is there any downside to this patch (is X86_CMPXCHG in the same sort of
> boat?)

Only if we don't use cmpxchg in shared memory with other domains or the
hypervisor.  (I don't think it will dynamically switch between real and
emulated cmpxchg depending on availability.)

    J

Re: [PATCH] xen: drop anti-dependency on X86_VISWS

[Jeremy Fitzhardinge]

On 04/06/2011 11:58 PM, Ian Campbell wrote:
> On Wed, 2011-04-06 at 22:45 +0100, David Miller wrote:
>> From: Ian Campbell <Ian.Campbell@eu.citrix.com>
>> Date: Mon, 4 Apr 2011 10:55:55 +0100
>>
>>> You mean the "!X86_VISWS" I presume? It doesn't make sense to me either.
>> No, I think 32-bit x86 allmodconfig elides XEN because of it's X86_TSC dependency.
> TSC is a real dependency of the Xen interfaces.

Not really.  The TSC register is a requirement, but that's going to be
present on any CPU which can boot Xen.  We don't need any of the
kernel's TSC machinery though.

    J

Re: [PATCH] xen: drop anti-dependency on X86_VISWS

[Ian Campbell]

On Wed, 2011-04-06 at 22:45 +0100, David Miller wrote:
> From: Ian Campbell <Ian.Campbell@eu.citrix.com>
> Date: Mon, 4 Apr 2011 10:55:55 +0100
> 
> > You mean the "!X86_VISWS" I presume? It doesn't make sense to me either.
> 
> No, I think 32-bit x86 allmodconfig elides XEN because of it's X86_TSC dependency.

TSC is a real dependency of the Xen interfaces.

> And, well, you could type "make allmodconfig" on your tree and see for
> yourself instead of asking me :-)

True.

X86_TSC not being enabled appears to due to CONFIG_ELAN being enabled
which causes the processor selection option (which defaults to M686,
which is a sane choice and enables TSC etc) to be gated at the top level
in arch/x86/Kconfig.cpu. Disabling the ELAN option then leaves X86_TSC
gated on !CONFIG_NUMAQ but removing that results in a generally useful
looking config.

It's a shame that these sorts of minority options cause allmodconfig to
omit support for more interesting configurations, such as modern
processors. Other than negating the semantics of such options I'm not
really sure what can be done about it though. On the other hand
compiling all the unusual stuff in an allmodconfig is probably a
positive thing.

I'm not sure why ELAN belongs in the EXTENDED_PLATFORM option space
rather than in the CPU choice option, since its only impact seems to be
on -march, MODULE_PROC_FAMILY and some cpufreq drivers which doesn't
sound like an extended platform to me but does it appear to be
deliberate (see 9e111f3e167a "x86: move ELAN to the
NON_STANDARD_PLATFORM section", that was the old name for
EXTENDED_PLATFORM).

Hrm, what about the following? (doesn't actually make a difference to
Xen since allmodconfig chooses HIGHMEM4G instead of HIGHMEM64G in the !
NUMAQ case but I stopped worrying about that several paragraphs ago)

8<--------

x86: invert X86_EXTENDED_PLATFORM to X86_STANDARD_PLATFORM

Having the =y choice be the more "standard" configuration causes
all*config to provide greater coverage of usual configurations.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index cc6c53a..6d8a404 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -299,15 +299,15 @@ config X86_BIGSMP
 	  This option is needed for the systems that have more than 8 CPUs
 
 if X86_32
-config X86_EXTENDED_PLATFORM
-	bool "Support for extended (non-PC) x86 platforms"
+config X86_STANDARD_PLATFORM
+	bool "Restrict support to standard (PC) x86 platforms"
 	default y
 	---help---
-	  If you disable this option then the kernel will only support
+	  If you enable this option then the kernel will only support
 	  standard PC platforms. (which covers the vast majority of
 	  systems out there.)
 
-	  If you enable this option then you'll be able to select support
+	  If you disable this option then you'll be able to select support
 	  for the following (non-PC) 32 bit x86 platforms:
 		AMD Elan
 		NUMAQ (IBM/Sequent)
@@ -318,25 +318,25 @@ config X86_EXTENDED_PLATFORM
 		Moorestown MID devices
 
 	  If you have one of these systems, or if you want to build a
-	  generic distribution kernel, say Y here - otherwise say N.
+	  generic distribution kernel, say N here - otherwise say Y.
 endif
 
 if X86_64
-config X86_EXTENDED_PLATFORM
-	bool "Support for extended (non-PC) x86 platforms"
+config X86_STANDARD_PLATFORM
+	bool "Restrict support to standard (PC) x86 platforms"
 	default y
 	---help---
-	  If you disable this option then the kernel will only support
+	  If you enable this option then the kernel will only support
 	  standard PC platforms. (which covers the vast majority of
 	  systems out there.)
 
-	  If you enable this option then you'll be able to select support
+	  If you disable this option then you'll be able to select support
 	  for the following (non-PC) 64 bit x86 platforms:
 		ScaleMP vSMP
 		SGI Ultraviolet
 
 	  If you have one of these systems, or if you want to build a
-	  generic distribution kernel, say Y here - otherwise say N.
+	  generic distribution kernel, say N here - otherwise say Y.
 endif
 # This is an alphabetically sorted list of 64 bit extended platforms
 # Please maintain the alphabetic order if and when there are additions
@@ -346,7 +346,7 @@ config X86_VSMP
 	select PARAVIRT_GUEST
 	select PARAVIRT
 	depends on X86_64 && PCI
-	depends on X86_EXTENDED_PLATFORM
+	depends on !X86_STANDARD_PLATFORM
 	---help---
 	  Support for ScaleMP vSMP systems.  Say 'Y' here if this kernel is
 	  supposed to run on these EM64T-based machines.  Only choose this option
@@ -355,7 +355,7 @@ config X86_VSMP
 config X86_UV
 	bool "SGI Ultraviolet"
 	depends on X86_64
-	depends on X86_EXTENDED_PLATFORM
+	depends on !X86_STANDARD_PLATFORM
 	depends on NUMA
 	depends on X86_X2APIC
 	---help---
@@ -368,7 +368,7 @@ config X86_UV
 config X86_ELAN
 	bool "AMD Elan"
 	depends on X86_32
-	depends on X86_EXTENDED_PLATFORM
+	depends on !X86_STANDARD_PLATFORM
 	---help---
 	  Select this for an AMD Elan processor.
 
@@ -381,7 +381,7 @@ config X86_INTEL_CE
 	depends on PCI
 	depends on PCI_GODIRECT
 	depends on X86_32
-	depends on X86_EXTENDED_PLATFORM
+	depends on !X86_STANDARD_PLATFORM
 	select X86_REBOOTFIXUPS
 	select OF
 	select OF_EARLY_FLATTREE
@@ -395,7 +395,7 @@ config X86_MRST
 	depends on PCI
 	depends on PCI_GOANY
 	depends on X86_32
-	depends on X86_EXTENDED_PLATFORM
+	depends on !X86_STANDARD_PLATFORM
 	depends on X86_IO_APIC
 	select APB_TIMER
 	select I2C
@@ -413,7 +413,7 @@ config X86_MRST
 config X86_RDC321X
 	bool "RDC R-321x SoC"
 	depends on X86_32
-	depends on X86_EXTENDED_PLATFORM
+	depends on !X86_STANDARD_PLATFORM
 	select M486
 	select X86_REBOOTFIXUPS
 	---help---
@@ -424,7 +424,7 @@ config X86_RDC321X
 config X86_32_NON_STANDARD
 	bool "Support non-standard 32-bit SMP architectures"
 	depends on X86_32 && SMP
-	depends on X86_EXTENDED_PLATFORM
+	depends on !X86_STANDARD_PLATFORM
 	---help---
 	  This option compiles in the NUMAQ, Summit, bigsmp, ES7000, default
 	  subarchitectures.  It is intended for a generic binary kernel.

Re: [PATCH] xen: drop anti-dependency on X86_VISWS

[David Miller]

From: Ian Campbell <Ian.Campbell@eu.citrix.com>
Date: Mon, 4 Apr 2011 10:55:55 +0100

> You mean the "!X86_VISWS" I presume? It doesn't make sense to me either.

No, I think 32-bit x86 allmodconfig elides XEN because of it's X86_TSC dependency.

And, well, you could type "make allmodconfig" on your tree and see for
yourself instead of asking me :-)

[PATCH] xen: drop anti-dependency on X86_VISWS (Was: Re: [PATCH] xen: netfront: fix declaration order)

[Ian Campbell]

On Mon, 2011-04-04 at 01:24 +0100, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Sun, 03 Apr 2011 13:07:19 +0200
> 
> > [PATCH] xen: netfront: fix declaration order
> > 
> > Must declare xennet_fix_features() and xennet_set_features() before
> > using them.
> > 
> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> > Cc: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> 
> Ugh, it makes no sense that XEN won't make it into the x86_32
> allmodconfig build.  Those dependencies in arch/x86/xen/Kconfig
> are terrible.

You mean the "!X86_VISWS" I presume? It doesn't make sense to me either.
Or at least I'm not sure why this single X86_32_NON_STANDARD machine is
more special than the others to require an anti-dependency like this.

It seems to have originally appeared from f0f32fccbffa on
CONFIG_PARAVIRT due to a conflict around ARCH_SETUP() and subsequently
got pushed down to CONFIG_XEN. However ARCH_SETUP doesn't exist any more
and I think the subarch stuff has been much improved since then so there
should be no conflict any more.

I dropped the dependency and, with a bit of fiddling, was able to build
a kernel with both CONFIG_X86_VISWS and CONFIG_XEN which booted as a Xen
domU.

tglx, Andrey, to get VISWS to build I had to comment out some code in
arch/x86/platform/visws/visws_quirks.c which seems to have been missed
during some irq_chip update or something?

          CC      arch/x86/platform/visws/visws_quirks.o
        arch/x86/platform/visws/visws_quirks.c: In function 'startup_piix4_master_irq':
        arch/x86/platform/visws/visws_quirks.c:474: warning: no return statement in function returning non-void
        arch/x86/platform/visws/visws_quirks.c: At top level:
        arch/x86/platform/visws/visws_quirks.c:495: error: unknown field 'mask' specified in initializer
        arch/x86/platform/visws/visws_quirks.c:495: warning: initialization from incompatible pointer type
        arch/x86/platform/visws/visws_quirks.c: In function 'set_piix4_virtual_irq_type':
        arch/x86/platform/visws/visws_quirks.c:583: error: 'struct irq_chip' has no member named 'enable'
        arch/x86/platform/visws/visws_quirks.c:583: error: 'struct irq_chip' has no member named 'unmask'
        arch/x86/platform/visws/visws_quirks.c:584: error: 'struct irq_chip' has no member named 'disable'
        arch/x86/platform/visws/visws_quirks.c:584: error: 'struct irq_chip' has no member named 'mask'
        arch/x86/platform/visws/visws_quirks.c:585: error: 'struct irq_chip' has no member named 'unmask'
        arch/x86/platform/visws/visws_quirks.c:585: error: 'struct irq_chip' has no member named 'unmask'
        arch/x86/platform/visws/visws_quirks.c: In function 'visws_pre_intr_init':
        arch/x86/platform/visws/visws_quirks.c:602: error: expected expression before '>' token
        make[4]: *** [arch/x86/platform/visws/visws_quirks.o] Error 1

Ian

8<--------

From db0ae26f479306ee8ebcfe2a08aa56a6dfe63987 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@citrix.com>
Date: Mon, 4 Apr 2011 10:27:47 +0100
Subject: [PATCH] xen: drop anti-dependency on X86_VISWS

This seems to have been added in f0f32fccbffa to avoid a conflict arising from
the long deceased ARCH_SETUP() macro and subsequently pushed down to the XEN
option.

As far as I can tell the conflict is no longer present and by dropping the
dependency I was able to build a kernel which has both CONFIG_XEN and
CONFIG_X86_VISWS enabled and boot it on Xen. I didn't try it on the VISWS
platform.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: konrad.wilk@oracle.com
Cc: xen-devel@lists.xensource.com
Cc: Randy Dunlap <randy.dunlap@oracle.com>
Cc: Andrey Panin <pazke@donpac.ru>
Cc: linux-visws-devel@lists.sf.net
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
---
 arch/x86/xen/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
index 1c7121b..65d7b13 100644
--- a/arch/x86/xen/Kconfig
+++ b/arch/x86/xen/Kconfig
@@ -6,7 +6,7 @@ config XEN
 	bool "Xen guest support"
 	select PARAVIRT
 	select PARAVIRT_CLOCK
-	depends on X86_64 || (X86_32 && X86_PAE && !X86_VISWS)
+	depends on X86_64 || (X86_32 && X86_PAE)
 	depends on X86_CMPXCHG && X86_TSC
 	help
 	  This is the Linux Xen port.  Enabling this will allow the
-- 
1.7.2.5




_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

[PATCH] xen: netfront: fix declaration order

[Eric Dumazet]

Le vendredi 01 avril 2011 à 20:54 -0700, David Miller a écrit :
> From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> Date: Thu, 31 Mar 2011 13:01:35 +0200 (CEST)
> 
> > Not tested in any way. The original code for offload setting seems broken
> > as it resets the features on every netback reconnect.
> > 
> > This will set GSO_ROBUST at device creation time (earlier than connect time).
> > 
> > RX checksum offload is forced on - so advertise as it is.
> > 
> > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> 
> Applied.

Hmm... I had to apply following patch to make it actually compile.

Thanks

[PATCH] xen: netfront: fix declaration order

Must declare xennet_fix_features() and xennet_set_features() before
using them.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/net/xen-netfront.c |   72 +++++++++++++++++------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index f6e7e27..0cfe4cc 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1140,6 +1140,42 @@ static void xennet_uninit(struct net_device *dev)
 	gnttab_free_grant_references(np->gref_rx_head);
 }
 
+static u32 xennet_fix_features(struct net_device *dev, u32 features)
+{
+	struct netfront_info *np = netdev_priv(dev);
+	int val;
+
+	if (features & NETIF_F_SG) {
+		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend, "feature-sg",
+				 "%d", &val) < 0)
+			val = 0;
+
+		if (!val)
+			features &= ~NETIF_F_SG;
+	}
+
+	if (features & NETIF_F_TSO) {
+		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
+				 "feature-gso-tcpv4", "%d", &val) < 0)
+			val = 0;
+
+		if (!val)
+			features &= ~NETIF_F_TSO;
+	}
+
+	return features;
+}
+
+static int xennet_set_features(struct net_device *dev, u32 features)
+{
+	if (!(features & NETIF_F_SG) && dev->mtu > ETH_DATA_LEN) {
+		netdev_info(dev, "Reducing MTU because no SG offload");
+		dev->mtu = ETH_DATA_LEN;
+	}
+
+	return 0;
+}
+
 static const struct net_device_ops xennet_netdev_ops = {
 	.ndo_open            = xennet_open,
 	.ndo_uninit          = xennet_uninit,
@@ -1513,42 +1549,6 @@ again:
 	return err;
 }
 
-static u32 xennet_fix_features(struct net_device *dev, u32 features)
-{
-	struct netfront_info *np = netdev_priv(dev);
-	int val;
-
-	if (features & NETIF_F_SG) {
-		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend, "feature-sg",
-				 "%d", &val) < 0)
-			val = 0;
-
-		if (!val)
-			features &= ~NETIF_F_SG;
-	}
-
-	if (features & NETIF_F_TSO) {
-		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
-				 "feature-gso-tcpv4", "%d", &val) < 0)
-			val = 0;
-
-		if (!val)
-			features &= ~NETIF_F_TSO;
-	}
-
-	return features;
-}
-
-static int xennet_set_features(struct net_device *dev, u32 features)
-{
-	if (!(features & NETIF_F_SG) && dev->mtu > ETH_DATA_LEN) {
-		netdev_info(dev, "Reducing MTU because no SG offload");
-		dev->mtu = ETH_DATA_LEN;
-	}
-
-	return 0;
-}
-
 static int xennet_connect(struct net_device *dev)
 {
 	struct netfront_info *np = netdev_priv(dev);


_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

Re: [PATCH RESEND] net: convert xen-netfront to hw_features

[Ian Campbell]

On Sat, 2011-04-02 at 04:54 +0100, David Miller wrote:
> From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> Date: Thu, 31 Mar 2011 13:01:35 +0200 (CEST)
> 
> > Not tested in any way. The original code for offload setting seems broken
> > as it resets the features on every netback reconnect.
> > 
> > This will set GSO_ROBUST at device creation time (earlier than connect time).
> > 
> > RX checksum offload is forced on - so advertise as it is.
> > 
> > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> 
> Applied.

Thanks, but unfortunately the patch results in the features all being
disabled by default, since they are not set in the initial dev->features
and the initial dev->wanted_features is based on features & hw_features.
The ndo_fix_features hook only clears features and doesn't add new
features (nor should it AFAICT).

Features cannot be negotiated with the backend until xennet_connect().
The carrier is not enabled until the end of that function, therefore I
think it is safe to start with a full set of features in dev->features
and rely on the call to netdev_update_features() in xennet_connect() to
clear those which turn out to be unavailable.

The following works for me, I guess the alternative is for
xennet_connect() to expand dev->features based on what it detects? Or is
there a mechanism for a driver to inform the core that a new hardware
feature has become available (I doubt that really happens on physical
h/w so I guess not).

Ian.

8<-----------------

From 0b56469abe56efae415b4603ef508ce9aec0e4c1 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@citrix.com>
Date: Mon, 4 Apr 2011 10:58:50 +0100
Subject: [PATCH] xen: netfront: assume all hw features are available until backend connection setup

We need to assume that all features will be available when registering the
netdev otherwise they are ommitted from the initial set of
dev->wanted_features. When we connect to the backed we reduce the set as
necessary due to the call to netdev_update_features() in xennet_connect().

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: mirq-linux@rere.qmqm.pl
Cc: netdev@vger.kernel.org <netdev@vger.kernel.org>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: konrad.wilk@oracle.com
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: xen-devel@lists.xensource.com
---
 drivers/net/xen-netfront.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 0cfe4cc..db9a763 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1251,6 +1251,14 @@ static struct net_device * __devinit xennet_create_dev(struct xenbus_device *dev
 				  NETIF_F_GSO_ROBUST;
 	netdev->hw_features	= NETIF_F_IP_CSUM | NETIF_F_SG | NETIF_F_TSO;
 
+	/*
+         * Assume that all hw features are available for now. This set
+         * will be adjusted by the call to netdev_update_features() in
+         * xennet_connect() which is the earliest point where we can
+         * negotiate with the backend regarding supported features.
+         */
+	netdev->features |= netdev->hw_features;
+
 	SET_ETHTOOL_OPS(netdev, &xennet_ethtool_ops);
 	SET_NETDEV_DEV(netdev, &dev->dev);
 
-- 
1.7.2.5



_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

Re: [PATCH RESEND] net: convert xen-netfront to hw_features

[Ian Campbell]

On Sat, 2011-04-02 at 04:54 +0100, David Miller wrote:
> From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> Date: Thu, 31 Mar 2011 13:01:35 +0200 (CEST)
> 
> > Not tested in any way. The original code for offload setting seems broken
> > as it resets the features on every netback reconnect.
> > 
> > This will set GSO_ROBUST at device creation time (earlier than connect time).
> > 
> > RX checksum offload is forced on - so advertise as it is.
> > 
> > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> 
> Applied.

Thanks, but unfortunately the patch results in the features all being
disabled by default, since they are not set in the initial dev->features
and the initial dev->wanted_features is based on features & hw_features.
The ndo_fix_features hook only clears features and doesn't add new
features (nor should it AFAICT).

Features cannot be negotiated with the backend until xennet_connect().
The carrier is not enabled until the end of that function, therefore I
think it is safe to start with a full set of features in dev->features
and rely on the call to netdev_update_features() in xennet_connect() to
clear those which turn out to be unavailable.

The following works for me, I guess the alternative is for
xennet_connect() to expand dev->features based on what it detects? Or is
there a mechanism for a driver to inform the core that a new hardware
feature has become available (I doubt that really happens on physical
h/w so I guess not).

Ian.

8<-----------------

>From 0b56469abe56efae415b4603ef508ce9aec0e4c1 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@citrix.com>
Date: Mon, 4 Apr 2011 10:58:50 +0100
Subject: [PATCH] xen: netfront: assume all hw features are available until backend connection setup

We need to assume that all features will be available when registering the
netdev otherwise they are ommitted from the initial set of
dev->wanted_features. When we connect to the backed we reduce the set as
necessary due to the call to netdev_update_features() in xennet_connect().

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: mirq-linux@rere.qmqm.pl
Cc: netdev@vger.kernel.org <netdev@vger.kernel.org>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: konrad.wilk@oracle.com
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: xen-devel@lists.xensource.com
---
 drivers/net/xen-netfront.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 0cfe4cc..db9a763 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1251,6 +1251,14 @@ static struct net_device * __devinit xennet_create_dev(struct xenbus_device *dev
 				  NETIF_F_GSO_ROBUST;
 	netdev->hw_features	= NETIF_F_IP_CSUM | NETIF_F_SG | NETIF_F_TSO;
 
+	/*
+         * Assume that all hw features are available for now. This set
+         * will be adjusted by the call to netdev_update_features() in
+         * xennet_connect() which is the earliest point where we can
+         * negotiate with the backend regarding supported features.
+         */
+	netdev->features |= netdev->hw_features;
+
 	SET_ETHTOOL_OPS(netdev, &xennet_ethtool_ops);
 	SET_NETDEV_DEV(netdev, &dev->dev);
 
-- 
1.7.2.5

Re: [PATCH RESEND] net: convert xen-netfront to hw_features

[David Miller]

From: Ian Campbell <Ian.Campbell@eu.citrix.com>
Date: Mon, 4 Apr 2011 13:29:19 +0100

>>From 0b56469abe56efae415b4603ef508ce9aec0e4c1 Mon Sep 17 00:00:00 2001
> From: Ian Campbell <ian.campbell@citrix.com>
> Date: Mon, 4 Apr 2011 10:58:50 +0100
> Subject: [PATCH] xen: netfront: assume all hw features are available until backend connection setup
> 
> We need to assume that all features will be available when registering the
> netdev otherwise they are ommitted from the initial set of
> dev->wanted_features. When we connect to the backed we reduce the set as
> necessary due to the call to netdev_update_features() in xennet_connect().
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

I've applied this, thanks Ian.

Re: [PATCH RESEND] net: convert xen-netfront to hw_features

[David Miller]

From: Ian Campbell <Ian.Campbell@eu.citrix.com>
Date: Mon, 4 Apr 2011 13:29:19 +0100

>>From 0b56469abe56efae415b4603ef508ce9aec0e4c1 Mon Sep 17 00:00:00 2001
> From: Ian Campbell <ian.campbell@citrix.com>
> Date: Mon, 4 Apr 2011 10:58:50 +0100
> Subject: [PATCH] xen: netfront: assume all hw features are available until backend connection setup
> 
> We need to assume that all features will be available when registering the
> netdev otherwise they are ommitted from the initial set of
> dev->wanted_features. When we connect to the backed we reduce the set as
> necessary due to the call to netdev_update_features() in xennet_connect().
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

I've applied this, thanks Ian.

Re: [PATCH RESEND] net: convert xen-netfront to hw_features

[David Miller]

From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Date: Thu, 31 Mar 2011 13:01:35 +0200 (CEST)

> Not tested in any way. The original code for offload setting seems broken
> as it resets the features on every netback reconnect.
> 
> This will set GSO_ROBUST at device creation time (earlier than connect time).
> 
> RX checksum offload is forced on - so advertise as it is.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

Applied.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

[PATCH RESEND] net: convert xen-netfront to hw_features

[Michał Mirosław]

Not tested in any way. The original code for offload setting seems broken
as it resets the features on every netback reconnect.

This will set GSO_ROBUST at device creation time (earlier than connect time).

RX checksum offload is forced on - so advertise as it is.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
[I don't know Xen code enough to say this is correct. There is Xen netback
driver coming in, that has similar changes to be made. Please match
them up if you can.]

 drivers/net/xen-netfront.c |   57 +++++++++++++++++--------------------------
 1 files changed, 23 insertions(+), 34 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 5c8d9c3..2a71c9f 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1148,6 +1148,8 @@ static const struct net_device_ops xennet_netdev_ops = {
 	.ndo_change_mtu	     = xennet_change_mtu,
 	.ndo_set_mac_address = eth_mac_addr,
 	.ndo_validate_addr   = eth_validate_addr,
+	.ndo_fix_features    = xennet_fix_features,
+	.ndo_set_features    = xennet_set_features,
 };
 
 static struct net_device * __devinit xennet_create_dev(struct xenbus_device *dev)
@@ -1209,7 +1211,9 @@ static struct net_device * __devinit xennet_create_dev(struct xenbus_device *dev
 	netdev->netdev_ops	= &xennet_netdev_ops;
 
 	netif_napi_add(netdev, &np->napi, xennet_poll, 64);
-	netdev->features        = NETIF_F_IP_CSUM;
+	netdev->features        = NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
+				  NETIF_F_GSO_ROBUST;
+	netdev->hw_features	= NETIF_F_IP_CSUM | NETIF_F_SG | NETIF_F_TSO;
 
 	SET_ETHTOOL_OPS(netdev, &xennet_ethtool_ops);
 	SET_NETDEV_DEV(netdev, &dev->dev);
@@ -1510,52 +1514,40 @@ again:
 	return err;
 }
 
-static int xennet_set_sg(struct net_device *dev, u32 data)
+static u32 xennet_fix_features(struct net_device *dev, u32 features)
 {
-	if (data) {
-		struct netfront_info *np = netdev_priv(dev);
-		int val;
+	struct netfront_info *np = netdev_priv(dev);
+	int val;
 
+	if (features & NETIF_F_SG) {
 		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend, "feature-sg",
 				 "%d", &val) < 0)
 			val = 0;
+
 		if (!val)
-			return -ENOSYS;
-	} else if (dev->mtu > ETH_DATA_LEN)
-		dev->mtu = ETH_DATA_LEN;
-
-	return ethtool_op_set_sg(dev, data);
-}
-
-static int xennet_set_tso(struct net_device *dev, u32 data)
-{
-	if (data) {
-		struct netfront_info *np = netdev_priv(dev);
-		int val;
+			features &= ~NETIF_F_SG;
+	}
 
+	if (features & NETIF_F_TSO) {
 		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
 				 "feature-gso-tcpv4", "%d", &val) < 0)
 			val = 0;
+
 		if (!val)
-			return -ENOSYS;
+			features &= ~NETIF_F_TSO;
 	}
 
-	return ethtool_op_set_tso(dev, data);
+	return features;
 }
 
-static void xennet_set_features(struct net_device *dev)
+static int xennet_set_features(struct net_device *dev, u32 features)
 {
-	/* Turn off all GSO bits except ROBUST. */
-	dev->features &= ~NETIF_F_GSO_MASK;
-	dev->features |= NETIF_F_GSO_ROBUST;
-	xennet_set_sg(dev, 0);
+	if (!(features & NETIF_F_SG) && dev->mtu > ETH_DATA_LEN) {
+		netdev_info(dev, "Reducing MTU because no SG offload");
+		dev->mtu = ETH_DATA_LEN;
+	}
 
-	/* We need checksum offload to enable scatter/gather and TSO. */
-	if (!(dev->features & NETIF_F_IP_CSUM))
-		return;
-
-	if (!xennet_set_sg(dev, 1))
-		xennet_set_tso(dev, 1);
+	return 0;
 }
 
 static int xennet_connect(struct net_device *dev)
@@ -1582,7 +1574,7 @@ static int xennet_connect(struct net_device *dev)
 	if (err)
 		return err;
 
-	xennet_set_features(dev);
+	netdev_update_features(dev);
 
 	spin_lock_bh(&np->rx_lock);
 	spin_lock_irq(&np->tx_lock);
@@ -1710,9 +1702,6 @@ static void xennet_get_strings(struct net_device *dev, u32 stringset, u8 * data)
 
 static const struct ethtool_ops xennet_ethtool_ops =
 {
-	.set_tx_csum = ethtool_op_set_tx_csum,
-	.set_sg = xennet_set_sg,
-	.set_tso = xennet_set_tso,
 	.get_link = ethtool_op_get_link,
 
 	.get_sset_count = xennet_get_sset_count,
-- 
1.7.2.5

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization