[PATCH 0/5]

[PATCH 0/5]

[Joshua Clayton]

The main goal of this series is to get rid of a needless and ugly typedef
in the rtl8712 wlan driver.

In the course of fixing that, I found a bug thati will can might (at least in theory)
lead to a overrun during a memcpy, as well as a duplicate struct.
Finally after cleaning up the typedef, I could not bring myself to leave
a variable called SupportedRates in the kernel with my name on it.

I have tested this on amd64. cwthe module loads and doesn't explode



Joshua Clayton (5):
  staging: rtl8712: fix buggy size calculation
  staging: rtl8712: simplify size calculation
  staging: rtl8712: remove duplicate struct
  staging: rtl8712: remove typedefs
  staging: rtl8712: style fix:

 drivers/staging/rtl8712/ieee80211.c           | 22 ++++++-------
 drivers/staging/rtl8712/rtl871x_cmd.c         | 28 +++++-----------
 drivers/staging/rtl8712/rtl871x_cmd.h         |  4 +--
 drivers/staging/rtl8712/rtl871x_event.h       |  2 +-
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 33 +++++++++----------
 drivers/staging/rtl8712/rtl871x_mlme.c        | 47 ++++++++++-----------------
 drivers/staging/rtl8712/rtl871x_mlme.h        |  2 +-
 drivers/staging/rtl8712/rtl871x_mp_ioctl.c    |  6 ++--
 drivers/staging/rtl8712/wlan_bssdef.h         | 32 +++---------------
 9 files changed, 63 insertions(+), 113 deletions(-)

-- 
2.4.6

[PATCH 1/5] staging: rtl8712: fix buggy size calculation

[Joshua Clayton]

r8712_get_ndis_wlan_bssid_ex_sz has a "6 * sizeof(unsigned long)"
where the underlying struct has a 6 * unsigned char.
Simplify the calculation by just subtracting the variable part from
the size of the struct.

This also gets rid of a use of typedef NDIS_802_11_RATES_EX

Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
 drivers/staging/rtl8712/rtl871x_mlme.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c
index c044b0e..6b3451f 100644
--- a/drivers/staging/rtl8712/rtl871x_mlme.c
+++ b/drivers/staging/rtl8712/rtl871x_mlme.c
@@ -210,17 +210,7 @@ void r8712_generate_random_ibss(u8 *pibss)
 
 uint r8712_get_ndis_wlan_bssid_ex_sz(struct ndis_wlan_bssid_ex *bss)
 {
-	uint t_len;
-
-	t_len = sizeof(u32) + 6 * sizeof(unsigned long) + 2 +
-			sizeof(struct ndis_802_11_ssid) + sizeof(u32) +
-			sizeof(s32) +
-			sizeof(enum NDIS_802_11_NETWORK_TYPE) +
-			sizeof(struct NDIS_802_11_CONFIGURATION) +
-			sizeof(enum NDIS_802_11_NETWORK_INFRASTRUCTURE) +
-			sizeof(NDIS_802_11_RATES_EX) +
-			sizeof(u32) + bss->IELength;
-	return t_len;
+	return sizeof(*bss) + bss->IELength - MAX_IE_SZ;
 }
 
 u8 *r8712_get_capability_from_ie(u8 *ie)
-- 
2.4.6

Re: [PATCH 1/5] staging: rtl8712: fix buggy size calculation

[Dan Carpenter]

On Thu, Jul 23, 2015 at 09:38:33PM -0700, Joshua Clayton wrote:
> r8712_get_ndis_wlan_bssid_ex_sz has a "6 * sizeof(unsigned long)"
> where the underlying struct has a 6 * unsigned char.
> Simplify the calculation by just subtracting the variable part from
> the size of the struct.
> 
> This also gets rid of a use of typedef NDIS_802_11_RATES_EX
> 
> Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>

Looks like you are right.

Reviewed-by: Dan Carpenter

regards,
dan carpenter

[PATCH 2/5] staging: rtl8712: simplify size calculation

[Joshua Clayton]

replace item-by-item  size calculation of a struct
with the size of the struct.

This gets rid of a use of typedef NDIS_802_11_RATES_EX

Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
 drivers/staging/rtl8712/rtl871x_cmd.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_cmd.c b/drivers/staging/rtl8712/rtl871x_cmd.c
index e35854d..f07050d 100644
--- a/drivers/staging/rtl8712/rtl871x_cmd.c
+++ b/drivers/staging/rtl8712/rtl871x_cmd.c
@@ -471,7 +471,6 @@ u8 r8712_createbss_cmd(struct _adapter *padapter)
 
 u8 r8712_joinbss_cmd(struct _adapter  *padapter, struct wlan_network *pnetwork)
 {
-	uint t_len = 0;
 	struct ndis_wlan_bssid_ex *psecnetwork;
 	struct cmd_obj		*pcmd;
 	struct cmd_priv		*pcmdpriv = &padapter->cmdpriv;
@@ -486,14 +485,6 @@ u8 r8712_joinbss_cmd(struct _adapter  *padapter, struct wlan_network *pnetwork)
 	pcmd = kmalloc(sizeof(*pcmd), GFP_ATOMIC);
 	if (pcmd == NULL)
 		return _FAIL;
-	t_len = sizeof(u32) + 6 * sizeof(unsigned char) + 2 +
-			sizeof(struct ndis_802_11_ssid) + sizeof(u32) +
-			sizeof(s32) +
-			sizeof(enum NDIS_802_11_NETWORK_TYPE) +
-			sizeof(struct NDIS_802_11_CONFIGURATION) +
-			sizeof(enum NDIS_802_11_NETWORK_INFRASTRUCTURE) +
-			sizeof(NDIS_802_11_RATES_EX) +
-			sizeof(u32) + MAX_IE_SZ;
 
 	/* for hidden ap to set fw_state here */
 	if (check_fwstate(pmlmepriv, WIFI_STATION_STATE|WIFI_ADHOC_STATE) !=
@@ -516,7 +507,7 @@ u8 r8712_joinbss_cmd(struct _adapter  *padapter, struct wlan_network *pnetwork)
 		kfree(pcmd);
 		return _FAIL;
 	}
-	memcpy(psecnetwork, &pnetwork->network, t_len);
+	memcpy(psecnetwork, &pnetwork->network, sizeof(*psecnetwork));
 	psecuritypriv->authenticator_ie[0] = (unsigned char)
 					     psecnetwork->IELength;
 	if ((psecnetwork->IELength-12) < (256 - 1))
-- 
2.4.6

[PATCH 4/5] staging: rtl8712: remove typedefs

[Joshua Clayton]

Coding style fix.
Get rid of typedefs NDIS_802_11_RATES and NDIS_802_11_RATES_EX

Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 4 ++--
 drivers/staging/rtl8712/wlan_bssdef.h         | 7 +------
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index 4f5f69c..087fbf1 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -635,7 +635,7 @@ static int r8711_wx_get_name(struct net_device *dev,
 	u8 ht_cap = false;
 	struct	mlme_priv	*pmlmepriv = &(padapter->mlmepriv);
 	struct wlan_bssid_ex *pcur_bss = &pmlmepriv->cur_network.network;
-	NDIS_802_11_RATES_EX *prates = NULL;
+	u8 *prates;
 
 	if (check_fwstate(pmlmepriv, _FW_LINKED|WIFI_ADHOC_MASTER_STATE) ==
 	    true) {
@@ -644,7 +644,7 @@ static int r8711_wx_get_name(struct net_device *dev,
 				 &ht_ielen, pcur_bss->IELength - 12);
 		if (p && ht_ielen > 0)
 			ht_cap = true;
-		prates = &pcur_bss->SupportedRates;
+		prates = pcur_bss->SupportedRates;
 		if (r8712_is_cckratesonly_included((u8 *)prates) == true) {
 			if (ht_cap == true)
 				snprintf(wrqu->name, IFNAMSIZ,
diff --git a/drivers/staging/rtl8712/wlan_bssdef.h b/drivers/staging/rtl8712/wlan_bssdef.h
index 7d769e8..7a410b5 100644
--- a/drivers/staging/rtl8712/wlan_bssdef.h
+++ b/drivers/staging/rtl8712/wlan_bssdef.h
@@ -32,11 +32,6 @@
 #define NDIS_802_11_LENGTH_RATES        8
 #define NDIS_802_11_LENGTH_RATES_EX     16
 
-/* Set of 8 data rates*/
-typedef unsigned char   NDIS_802_11_RATES[NDIS_802_11_LENGTH_RATES];
-/* Set of 16 data rates */
-typedef unsigned char   NDIS_802_11_RATES_EX[NDIS_802_11_LENGTH_RATES_EX];
-
 struct ndis_802_11_ssid {
 	u32 SsidLength;
 	u8  Ssid[32];
@@ -104,7 +99,7 @@ struct wlan_bssid_ex {
 	enum NDIS_802_11_NETWORK_TYPE  NetworkTypeInUse;
 	struct NDIS_802_11_CONFIGURATION  Configuration;
 	enum NDIS_802_11_NETWORK_INFRASTRUCTURE  InfrastructureMode;
-	NDIS_802_11_RATES_EX  SupportedRates;
+	u8 SupportedRates[NDIS_802_11_LENGTH_RATES_EX];
 	u32 IELength;
 	/*(timestamp, beacon interval, and capability information) */
 	u8 IEs[MAX_IE_SZ];
-- 
2.4.6

[PATCH 5/5] staging: rtl8712: style fix:

[Joshua Clayton]

change instances SupportedRates to compliant and sane "rates"
This change in no way harms readability, and brings several lines
under the 80 character limit.

Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
 drivers/staging/rtl8712/ieee80211.c           | 22 +++++++++++-----------
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 21 +++++++++------------
 drivers/staging/rtl8712/wlan_bssdef.h         |  4 ++--
 3 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/rtl8712/ieee80211.c b/drivers/staging/rtl8712/ieee80211.c
index 5786808..ca9dcaf 100644
--- a/drivers/staging/rtl8712/ieee80211.c
+++ b/drivers/staging/rtl8712/ieee80211.c
@@ -134,22 +134,22 @@ u8 *r8712_get_ie(u8 *pbuf, sint index, sint *len, sint limit)
 	return NULL;
 }
 
-static void set_supported_rate(u8 *SupportedRates, uint mode)
+static void set_supported_rate(u8 *rates, uint mode)
 {
-	memset(SupportedRates, 0, NDIS_802_11_LENGTH_RATES_EX);
+	memset(rates, 0, NDIS_802_11_LENGTH_RATES_EX);
 	switch (mode) {
 	case WIRELESS_11B:
-		memcpy(SupportedRates, WIFI_CCKRATES,
+		memcpy(rates, WIFI_CCKRATES,
 			IEEE80211_CCK_RATE_LEN);
 		break;
 	case WIRELESS_11G:
 	case WIRELESS_11A:
-		memcpy(SupportedRates, WIFI_OFDMRATES,
+		memcpy(rates, WIFI_OFDMRATES,
 			IEEE80211_NUM_OFDM_RATESLEN);
 		break;
 	case WIRELESS_11BG:
-		memcpy(SupportedRates, WIFI_CCKRATES, IEEE80211_CCK_RATE_LEN);
-		memcpy(SupportedRates + IEEE80211_CCK_RATE_LEN, WIFI_OFDMRATES,
+		memcpy(rates, WIFI_CCKRATES, IEEE80211_CCK_RATE_LEN);
+		memcpy(rates + IEEE80211_CCK_RATE_LEN, WIFI_OFDMRATES,
 			IEEE80211_NUM_OFDM_RATESLEN);
 		break;
 	}
@@ -195,17 +195,17 @@ int r8712_generate_ie(struct registry_priv *pregistrypriv)
 	ie = r8712_set_ie(ie, _SSID_IE_, pdev_network->Ssid.SsidLength,
 		    pdev_network->Ssid.Ssid, &sz);
 	/*supported rates*/
-	set_supported_rate(pdev_network->SupportedRates,
+	set_supported_rate(pdev_network->rates,
 			   pregistrypriv->wireless_mode);
-	rateLen = r8712_get_rateset_len(pdev_network->SupportedRates);
+	rateLen = r8712_get_rateset_len(pdev_network->rates);
 	if (rateLen > 8) {
 		ie = r8712_set_ie(ie, _SUPPORTEDRATES_IE_, 8,
-			    pdev_network->SupportedRates, &sz);
+			    pdev_network->rates, &sz);
 		ie = r8712_set_ie(ie, _EXT_SUPPORTEDRATES_IE_, (rateLen - 8),
-			    (pdev_network->SupportedRates + 8), &sz);
+			    (pdev_network->rates + 8), &sz);
 	} else
 		ie = r8712_set_ie(ie, _SUPPORTEDRATES_IE_,
-			    rateLen, pdev_network->SupportedRates, &sz);
+			    rateLen, pdev_network->rates, &sz);
 	/*DS parameter set*/
 	ie = r8712_set_ie(ie, _DSSET_IE_, 1,
 		    (u8 *)&(pdev_network->Configuration.DSConfig), &sz);
diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index 087fbf1..219239c 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -203,14 +203,12 @@ static inline char *translate_scan(struct _adapter *padapter,
 	}
 	/* Add the protocol name */
 	iwe.cmd = SIOCGIWNAME;
-	if ((r8712_is_cckratesonly_included((u8 *)&pnetwork->network.
-	     SupportedRates)) == true) {
+	if (r8712_is_cckratesonly_included(pnetwork->network.rates)) {
 		if (ht_cap == true)
 			snprintf(iwe.u.name, IFNAMSIZ, "IEEE 802.11bn");
 		else
 			snprintf(iwe.u.name, IFNAMSIZ, "IEEE 802.11b");
-	} else if ((r8712_is_cckrates_included((u8 *)&pnetwork->network.
-		    SupportedRates)) == true) {
+	} else if (r8712_is_cckrates_included(pnetwork->network.rates)) {
 		if (ht_cap == true)
 			snprintf(iwe.u.name, IFNAMSIZ, "IEEE 802.11bgn");
 		else
@@ -270,9 +268,9 @@ static inline char *translate_scan(struct _adapter *padapter,
 	iwe.u.bitrate.disabled = 0;
 	iwe.u.bitrate.value = 0;
 	i = 0;
-	while (pnetwork->network.SupportedRates[i] != 0) {
+	while (pnetwork->network.rates[i] != 0) {
 		/* Bit rate given in 500 kb/s units */
-		iwe.u.bitrate.value = (pnetwork->network.SupportedRates[i++] &
+		iwe.u.bitrate.value = (pnetwork->network.rates[i++] &
 				      0x7F) * 500000;
 		current_val = iwe_stream_add_value(info, start, current_val,
 			      stop, &iwe, IW_EV_PARAM_LEN);
@@ -644,15 +642,15 @@ static int r8711_wx_get_name(struct net_device *dev,
 				 &ht_ielen, pcur_bss->IELength - 12);
 		if (p && ht_ielen > 0)
 			ht_cap = true;
-		prates = pcur_bss->SupportedRates;
-		if (r8712_is_cckratesonly_included((u8 *)prates) == true) {
+		prates = pcur_bss->rates;
+		if (r8712_is_cckratesonly_included(prates) == true) {
 			if (ht_cap == true)
 				snprintf(wrqu->name, IFNAMSIZ,
 					 "IEEE 802.11bn");
 			else
 				snprintf(wrqu->name, IFNAMSIZ,
 					 "IEEE 802.11b");
-		} else if ((r8712_is_cckrates_included((u8 *)prates)) == true) {
+		} else if ((r8712_is_cckrates_included(prates)) == true) {
 			if (ht_cap == true)
 				snprintf(wrqu->name, IFNAMSIZ,
 					 "IEEE 802.11bgn");
@@ -1444,9 +1442,8 @@ static int r8711_wx_get_rate(struct net_device *dev,
 				    (IEEE80211_HT_CAP_SGI_20 |
 				    IEEE80211_HT_CAP_SGI_40)) ? 1 : 0;
 		}
-		while ((pcur_bss->SupportedRates[i] != 0) &&
-			(pcur_bss->SupportedRates[i] != 0xFF)) {
-			rate = pcur_bss->SupportedRates[i] & 0x7F;
+		while (pcur_bss->rates[i] && (pcur_bss->rates[i] != 0xFF)) {
+			rate = pcur_bss->rates[i] & 0x7F;
 			if (rate > max_rate)
 				max_rate = rate;
 			wrqu->bitrate.fixed = 0;	/* no auto select */
diff --git a/drivers/staging/rtl8712/wlan_bssdef.h b/drivers/staging/rtl8712/wlan_bssdef.h
index 7a410b5..8e2d798 100644
--- a/drivers/staging/rtl8712/wlan_bssdef.h
+++ b/drivers/staging/rtl8712/wlan_bssdef.h
@@ -83,7 +83,7 @@ struct NDIS_802_11_FIXED_IEs {
  * 6 * sizeof (unsigned char) + 2 + sizeof (ndis_802_11_ssid) + sizeof (u32)
  * + sizeof (s32) + sizeof (NDIS_802_11_NETWORK_TYPE)
  * + sizeof (struct NDIS_802_11_CONFIGURATION)
- * + sizeof (NDIS_802_11_RATES_EX) + IELength
+ * + sizeof (rates) + IELength
 
  * Except the IELength, all other fields are fixed length. Therefore, we can
  * define a macro to present the partial sum.
@@ -99,7 +99,7 @@ struct wlan_bssid_ex {
 	enum NDIS_802_11_NETWORK_TYPE  NetworkTypeInUse;
 	struct NDIS_802_11_CONFIGURATION  Configuration;
 	enum NDIS_802_11_NETWORK_INFRASTRUCTURE  InfrastructureMode;
-	u8 SupportedRates[NDIS_802_11_LENGTH_RATES_EX];
+	u8 rates[NDIS_802_11_LENGTH_RATES_EX];
 	u32 IELength;
 	/*(timestamp, beacon interval, and capability information) */
 	u8 IEs[MAX_IE_SZ];
-- 
2.4.6

Re: [PATCH 5/5] staging: rtl8712: style fix:

[Dan Carpenter]

Write a better subject line.

On Thu, Jul 23, 2015 at 09:53:18PM -0700, Joshua Clayton wrote:
> change instances SupportedRates to compliant and sane "rates"
> This change in no way harms readability, and brings several lines
> under the 80 character limit.
> 

Yeah, but it does a some other stuff as well like removing casts.

> -		while ((pcur_bss->SupportedRates[i] != 0) &&
> -			(pcur_bss->SupportedRates[i] != 0xFF)) {
> -			rate = pcur_bss->SupportedRates[i] & 0x7F;
> +		while (pcur_bss->rates[i] && (pcur_bss->rates[i] != 0xFF)) {
> +			rate = pcur_bss->rates[i] & 0x7F;
>  			if (rate > max_rate)
>  				max_rate = rate;
>  			wrqu->bitrate.fixed = 0;	/* no auto select */

I actually like the != 0 here because we're talking about the number
zero.  It should look like this:

		while (pcur_bss->rates[i] != 0 &&
		       pcur_bss->rates[i] != 0xFF) {

But removing the parens is something for a different patch.  I use a
script to help review these so when you mix different changes together
it means there is more manual review work for me.

regards,
dan carpenter

Re: [PATCH 5/5] staging: rtl8712: style fix:

[Joshua Clayton]

Dan,
On Friday, July 24, 2015 01:52:27 PM Dan Carpenter wrote:
> Write a better subject line.
> 
> On Thu, Jul 23, 2015 at 09:53:18PM -0700, Joshua Clayton wrote:
> > change instances SupportedRates to compliant and sane "rates"
> > This change in no way harms readability, and brings several lines
> > under the 80 character limit.
> 
> Yeah, but it does a some other stuff as well like removing casts.
I apologize. I thought it would make sense to double up (while I'm in there 
I'll just...).

> 
> > -		while ((pcur_bss->SupportedRates[i] != 0) &&
> > -			(pcur_bss->SupportedRates[i] != 0xFF)) {
> > -			rate = pcur_bss->SupportedRates[i] & 0x7F;
> > +		while (pcur_bss->rates[i] && (pcur_bss->rates[i] != 0xFF)) {
> > +			rate = pcur_bss->rates[i] & 0x7F;
> > 
> >  			if (rate > max_rate)
> >  			
> >  				max_rate = rate;
> >  			
> >  			wrqu->bitrate.fixed = 0;	/* no auto select */
> 
> I actually like the != 0 here because we're talking about the number
> zero.  It should look like this:
> 
> 		while (pcur_bss->rates[i] != 0 &&
> 		       pcur_bss->rates[i] != 0xFF) {
OK.

> 
> But removing the parens is something for a different patch.  I use a
> script to help review these so when you mix different changes together
> it means there is more manual review work for me.
Sorry about that.
I'll split or drop it in the next version
> 
> regards,
> dan carpenter

Joshua

[PATCH 0/5]

[Jason Wang]

Hi all:

This series tries to support batched IOTLB updating vhost-vdpa.

Currently vhost-vdpa accepts userspace mapping via IOTLB API, and it
can only forward one mapping to IOMMU or device through IOMMU API or
dma_map(). Though set_map() is designed to have the capability to pass
an rbtree based mapping to vDPA device, it's still be called at least
once for each VHOST_IOTLB_UPDATE or VHOST_IOTLB_INVALIDATE. This is
because vhost-vdpa doesn't know the userspace start or stop then
updating.

So this patch introduces two flags as hints for vhost-vdpa to call
set_map() only when userspace finish a batch of IOTLB updating.

So instead of:

1) VHOST_IOTLB_UPDATE/VHOST_IOTLB_INVALIDATE -> set_map() (s)
2) VHOST_IOTLB_UPDATE/VHOST_IOTLB_INVALIDATE -> set_map() (s)
...
n) VHOST_IOTLB_UPDATE/VHOST_IOTLB_INVALIDATE -> set_map() (s)

With the help of hints, we do:

0) VHOST_IOTLB_BATCH_START

1) VHOST_IOTLB_UPDATE/INVALIDATE
...
n) VHOST_IOTLB_UPDATE/INVALIDATE

n+1) VHOST_IOTLB_BATCH_END -> set_map()

One one call of set_map() to vDPA device for a batch of IOTLB
mappings. So for the device that has its own DMA translation logic, it
can efficiently structure the memory mapping to get best performance.

Note, this only impacts the devices that want its own DMA
translation (less times of set_map() call). For other type of devices,
there's no changes in the behaviour.

Changes from RFCV1:

- tweak the comments per Michael's request

Jason Wang (5):
  vhost-vdpa: refine ioctl pre-processing
  vhost: generialize backend features setting/getting
  vhost-vdpa: support get/set backend features
  vhost-vdpa: support IOTLB batching hints
  vdpasim: support batch updating

 drivers/vdpa/vdpa_sim/vdpa_sim.c | 40 +++++++++++++++++++++--
 drivers/vhost/net.c              | 18 ++---------
 drivers/vhost/vdpa.c             | 55 ++++++++++++++++++++++++++------
 drivers/vhost/vhost.c            | 15 +++++++++
 drivers/vhost/vhost.h            |  2 ++
 include/uapi/linux/vhost.h       |  2 ++
 include/uapi/linux/vhost_types.h | 11 +++++++
 7 files changed, 114 insertions(+), 29 deletions(-)

-- 
2.20.1

[PATCH 0/5]

[kusumi.tomohiro]

From: Tomohiro Kusumi <tkusumi@tuxera.com>

These are another cleanup patches to make it less OS dependent
(less Linux dependent).

Tomohiro Kusumi (5):
  Move {is,load}_blktrace() to a new header blktrace.h
  Drop struct thread_data dependency from os headers
  Drop circular dependency in log.c and lib/output_buffer.c
  Move Linux/ppc64 specific cpu_online() to os/os-linux.h
  Include sg headers in os/os-linux.h

 blktrace.c          |  1 +
 blktrace.h          | 23 +++++++++++++++++++++++
 fio.h               |  8 --------
 init.c              | 10 +++++++---
 iolog.c             |  1 +
 lib/output_buffer.c |  8 +-------
 lib/output_buffer.h |  2 +-
 log.c               |  6 ++++++
 os/os-linux.h       | 10 ++++++++++
 os/os-windows.h     |  4 +---
 os/os.h             | 34 +---------------------------------
 stat.c              |  6 ++++--
 12 files changed, 56 insertions(+), 57 deletions(-)
 create mode 100644 blktrace.h

-- 
2.9.4

Re: [PATCH 0/5]

[Jens Axboe]

On Tue, May 23 2017, kusumi.tomohiro@gmail.com wrote:
> From: Tomohiro Kusumi <tkusumi@tuxera.com>
> 
> These are another cleanup patches to make it less OS dependent
> (less Linux dependent).
> 
> Tomohiro Kusumi (5):
>   Move {is,load}_blktrace() to a new header blktrace.h
>   Drop struct thread_data dependency from os headers
>   Drop circular dependency in log.c and lib/output_buffer.c
>   Move Linux/ppc64 specific cpu_online() to os/os-linux.h
>   Include sg headers in os/os-linux.h

Looks good, applied, thanks.

-- 
Jens Axboe

[PATCH 0/5]

[minyard]

This is a small set of patches for the i801 I2C driver that are
mostly cleanups and consolidation.  No real behaviour change, except
not enabling PEC when it's not needed.

I have tested these pretty extensively on qemu with various configurations
and errors (I've hacked up qemu a bit to do this) and it seems to be ok.

Not sure if you are interested, but thought I would offer.

-corey

[PATCH 0/5]

[Liu Yu]

The whole patchset includes:
patch 1: fix kvmppc build error
patch 2: fix kvmppc init error
patch 3~5: add kvmppc guest debug support

The guest debug still have some problems I haven't solved.

1. gdb 'next' command uses software breakpoint
software breakpoint is implemented via modify guest's code.
In most case it works well,
but when used by 'next' it's easy to make trouble on powerpc booke.

For example booke has a code template for
jumping to and returning from interrupt handlers:

	bl transfer
	.long handler_addr
	.long ret_addr

when call transfer, it never return but
in transfer assembly code it will read the handler_addr
and ultimately call the handler.
Gdb doesn't know that and treat it as a normal function call.
so gdb put a software breakpoint instruction at handler_addr,
in order to get trap there when return from transfer.

Then guest will read software breakpoint as handler_addr and jump to there..

I'm not sure if x86 suffer this kind of issue.
Is there any way to avoid this?


2. gdb 'watch' command
Jan told me gdb>6.8 can issue hardware watchpoint request via command 'watch',
my gdb is 6.8.50.20080821-cvs and our toolchain provider confirm that it supports hardware watch
However when I use 'watch', I can only see single step from gdbstub side.
Did I miss anything?

Re: [PATCH 0/5]

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2108 bytes --]

Liu Yu wrote:
> The whole patchset includes:
> patch 1: fix kvmppc build error
> patch 2: fix kvmppc init error
> patch 3~5: add kvmppc guest debug support
> 
> The guest debug still have some problems I haven't solved.
> 
> 1. gdb 'next' command uses software breakpoint
> software breakpoint is implemented via modify guest's code.
> In most case it works well,
> but when used by 'next' it's easy to make trouble on powerpc booke.
> 
> For example booke has a code template for
> jumping to and returning from interrupt handlers:
> 
> 	bl transfer
> 	.long handler_addr
> 	.long ret_addr
> 
> when call transfer, it never return but
> in transfer assembly code it will read the handler_addr
> and ultimately call the handler.
> Gdb doesn't know that and treat it as a normal function call.
> so gdb put a software breakpoint instruction at handler_addr,
> in order to get trap there when return from transfer.
> 
> Then guest will read software breakpoint as handler_addr and jump to there..
> 
> I'm not sure if x86 suffer this kind of issue.

It would if it had such a pattern.

> Is there any way to avoid this?

Unless there is a mechanism via the debug infos of a binary to tell gdb
about this, I think one can only avoid it by not using next here.

> 
> 
> 2. gdb 'watch' command
> Jan told me gdb>6.8 can issue hardware watchpoint request via command 'watch',
> my gdb is 6.8.50.20080821-cvs and our toolchain provider confirm that it supports hardware watch
> However when I use 'watch', I can only see single step from gdbstub side.
> Did I miss anything?

Did you install a watchpoint on a symbol? If yes, try if placing one on
an absolute address changes the picture.

Frankly, I didn't understand gdb's logic for selecting soft or hard
watchpoints so far. Soft watchpoints are those you saw: single step to
the program, checking after each step if the watched variable has
changed. In theory it should be clear when to use which. But practice
appears to be non-deterministic, at least with the versions we recently
tried on x86.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

RE: [PATCH 0/5]

[Liu Yu-B13201]

 

> -----Original Message-----
> From: jan.kiszka@web.de [mailto:jan.kiszka@web.de] 
> Sent: Saturday, July 25, 2009 6:44 PM
> To: Liu Yu-B13201
> Cc: qemu-devel; Hollis Blanchard; kvm-ppc; Nathan Froyd
> Subject: Re: [PATCH 0/5]
> 
> Liu Yu wrote:
> > 2. gdb 'watch' command
> > Jan told me gdb>6.8 can issue hardware watchpoint request 
> via command 'watch',
> > my gdb is 6.8.50.20080821-cvs and our toolchain provider 
> confirm that it supports hardware watch
> > However when I use 'watch', I can only see single step from 
> gdbstub side.
> > Did I miss anything?
> 
> Did you install a watchpoint on a symbol? If yes, try if 
> placing one on
> an absolute address changes the picture.

Cool, it did use hardware watch when I used absolute address.
Seems I need to test more. :)

Re: [PATCH 0/5]

[Nathan Froyd]

On Sat, Jul 25, 2009 at 04:40:12PM +0800, Liu Yu wrote:
> For example booke has a code template for
> jumping to and returning from interrupt handlers:
>
> 	bl transfer
> 	.long handler_addr
> 	.long ret_addr
>
> when call transfer, it never return but
> in transfer assembly code it will read the handler_addr
> and ultimately call the handler.
> Gdb doesn't know that and treat it as a normal function call.
> so gdb put a software breakpoint instruction at handler_addr,
> in order to get trap there when return from transfer.
>
> Then guest will read software breakpoint as handler_addr and jump to there..
>
> I'm not sure if x86 suffer this kind of issue.
> Is there any way to avoid this?

You would need to modify GDB to recognize this sort of case with the
skip_trampoline_code gdbarch method.

-Nathan

RE: [PATCH 0/5]

[Liu Yu-B13201]

 

> -----Original Message-----
> From: Nathan Froyd [mailto:froydnj@codesourcery.com] 
> Sent: Monday, July 27, 2009 9:14 PM
> To: Liu Yu-B13201
> Cc: qemu-devel@nongnu.org; hollisb@us.ibm.com; 
> kvm-ppc@vger.kernel.org; jan.kiszka@siemens.com
> Subject: Re: [PATCH 0/5]
> 
> On Sat, Jul 25, 2009 at 04:40:12PM +0800, Liu Yu wrote:
> > For example booke has a code template for
> > jumping to and returning from interrupt handlers:
> >
> > 	bl transfer
> > 	.long handler_addr
> > 	.long ret_addr
> >
> > when call transfer, it never return but
> > in transfer assembly code it will read the handler_addr
> > and ultimately call the handler.
> > Gdb doesn't know that and treat it as a normal function call.
> > so gdb put a software breakpoint instruction at handler_addr,
> > in order to get trap there when return from transfer.
> >
> > Then guest will read software breakpoint as handler_addr 
> and jump to there..
> >
> > I'm not sure if x86 suffer this kind of issue.
> > Is there any way to avoid this?
> 
> You would need to modify GDB to recognize this sort of case with the
> skip_trampoline_code gdbarch method.
> 

Hmm.. I am not a gdb expert.
But even gdb can recognize this pattern, is it safe to skip it?

Re: [PATCH 0/5]

[Nathan Froyd]

On Tue, Jul 28, 2009 at 04:11:57PM +0800, Liu Yu-B13201 wrote:
> > On Sat, Jul 25, 2009 at 04:40:12PM +0800, Liu Yu wrote:
> > > For example booke has a code template for
> > > jumping to and returning from interrupt handlers:
> > >
> > > 	bl transfer
> > > 	.long handler_addr
> > > 	.long ret_addr
> > >
> > > when call transfer, it never return but
> > > in transfer assembly code it will read the handler_addr
> > > and ultimately call the handler.
> > > Gdb doesn't know that and treat it as a normal function call.
> > > so gdb put a software breakpoint instruction at handler_addr,
> > > in order to get trap there when return from transfer.
> > >
> > > Then guest will read software breakpoint as handler_addr 
> > and jump to there..
> > >
> > > I'm not sure if x86 suffer this kind of issue.
> > > Is there any way to avoid this?
> > 
> > You would need to modify GDB to recognize this sort of case with the
> > skip_trampoline_code gdbarch method.
> 
> Hmm.. I am not a gdb expert.
> But even gdb can recognize this pattern, is it safe to skip it?

The code doesn't get skipped.  skip_trampoline_code is a hook for
telling GDB "this function doesn't return in the normal way: here's
where execution will resume once this function finishes."  That way GDB
can place the software breakpoint in the correct location: in this case,
at the address handler_addr.

-Nathan

[PATCH 0/5]

[Luiz Fernando N. Capitulino]

 Hi,

 Some random things I've fixed while reading the code, they're
unrelated changes, but I think it's better to send in one shot.

Diff stat:

 Documentation/core-tutorial.txt |    9 ++++-----
 builtin-commit-tree.c           |    3 +--
 cache.h                         |    2 +-
 entry.c                         |   14 +++++++-------
 read-cache.c                    |   16 ++++++++--------
 5 files changed, 21 insertions(+), 23 deletions(-)

Re: [PATCH 0/5]

[Luiz Fernando N. Capitulino]

Em Tue, 24 Apr 2007 15:01:10 -0300
"Luiz Fernando N. Capitulino" <lcapitulino@mandriva.com.br> escreveu:

|  Hi,
| 
|  Some random things I've fixed while reading the code, they're
| unrelated changes, but I think it's better to send in one shot.

 Sorry for that, looks like git-send-email doesn't want to work
for me.

 Will send again shortly.

-- 
Luiz Fernando N. Capitulino

Re: [PATCH 0/5]

[Junio C Hamano]

I suspect that you need to quote your human readable name, as it
has a full-stop in it ("N.").  Maybe git-send-email should be
taught about it.

Re: [PATCH 0/5]

[Luiz Fernando N. Capitulino]

Em Tue, 24 Apr 2007 13:56:11 -0700
Junio C Hamano <junkio@cox.net> escreveu:

| I suspect that you need to quote your human readable name, as it
| has a full-stop in it ("N.").  Maybe git-send-email should be
| taught about it.

 It worked when I tested it sending the series to me and CC'ing
my gmail address.

 Will send this series by hand and figure out a better way to
do this...

-- 
Luiz Fernando N. Capitulino

Re: [PATCH 0/5]

[Hermes Trismegisto]

"Luiz Fernando N. Capitulino" <lcapitulino@mandriva.com.br>
writes:

> Em Tue, 24 Apr 2007 13:56:11 -0700
> Junio C Hamano <junkio@cox.net> escreveu:
>
> | I suspect that you need to quote your human readable name, as it
> | has a full-stop in it ("N.").  Maybe git-send-email should be
> | taught about it.
>
>  It worked when I tested it sending the series to me and CC'ing
> my gmail address.

Hmmm.  Maybe vger and my ISP have stricter filter based on set
of addresses that appear on To/Cc lines than the way you receive
mails?

In git-send-email, I notice that $author_name address on From:
line is quoted in dq pairs (in sub send_message) but none of the
addresses in @cc taken from the Signed-off-by lines are not
inspected nor quoted.  I do not know what is being done to To:
recipients offhand and I am too lazy to check.

>  Will send this series by hand and figure out a better way to
> do this...

Thanks.  They look reasonable, at least from my cursory look.

Re: [PATCH 0/5]

[Luiz Fernando N. Capitulino]

Em Wed, 25 Apr 2007 12:04:32 -0700
Hermes Trismegisto <junkio@cox.net> escreveu:

| "Luiz Fernando N. Capitulino" <lcapitulino@mandriva.com.br>
| writes:
| 
| > Em Tue, 24 Apr 2007 13:56:11 -0700
| > Junio C Hamano <junkio@cox.net> escreveu:
| >
| > | I suspect that you need to quote your human readable name, as it
| > | has a full-stop in it ("N.").  Maybe git-send-email should be
| > | taught about it.
| >
| >  It worked when I tested it sending the series to me and CC'ing
| > my gmail address.
| 
| Hmmm.  Maybe vger and my ISP have stricter filter based on set
| of addresses that appear on To/Cc lines than the way you receive
| mails?

 Maybe, but I've alreays used quilt send to submit patches and it
works quite well (not sure whether I've submitted more than five
patches to LKML though).

| In git-send-email, I notice that $author_name address on From:
| line is quoted in dq pairs (in sub send_message) but none of the
| addresses in @cc taken from the Signed-off-by lines are not
| inspected nor quoted.  I do not know what is being done to To:
| recipients offhand and I am too lazy to check.

 Heh, I can't check either.

| >  Will send this series by hand and figure out a better way to
| > do this...
| 
| Thanks.  They look reasonable, at least from my cursory look.

 You're welcome.

-- 
Luiz Fernando N. Capitulino