RE: [PATCH 3/3] tools/hv: Fix permissions of created directory and files

[KY Srinivasan]

> -Original Message-
> From: Tomas Hozza [mailto:tho...@redhat.com]
> Sent: Monday, November 12, 2012 3:55 AM
> To: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com; KY Srinivasan; b...@decadent.org.uk
> Cc: Tomas Hozza
> Subject: [PATCH 3/3] tools/hv: Fix permissions of created directory and files
> 
> From: Ben Hutchings 
> 
> It's silly to create directories without execute permission, or to
> give permissions to 'other' but not the group-owner.
> 
> Write the permissions in octal and 'ls -l' format since these are much
> easier to read than the named macros.
> 
> Signed-off-by: Ben Hutchings 
> Signed-off-by: Tomas Hozza 
Acked-by:  K. Y. Srinivasan 

> ---
>  tools/hv/hv_kvp_daemon.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index a581b3f..17703c7 100644
> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -236,7 +236,7 @@ static int kvp_file_init(void)
>   int alloc_unit = sizeof(struct kvp_record) * ENTRIES_PER_BLOCK;
> 
>   if (access(KVP_CONFIG_LOC, F_OK)) {
> - if (mkdir(KVP_CONFIG_LOC, S_IRUSR | S_IWUSR | S_IROTH)) {
> + if (mkdir(KVP_CONFIG_LOC, 0755 /* rwxr-xr-x */)) {
>   syslog(LOG_ERR, " Failed to create %s",
> KVP_CONFIG_LOC);
>   exit(EXIT_FAILURE);
>   }
> @@ -247,7 +247,7 @@ static int kvp_file_init(void)
>   records_read = 0;
>   num_blocks = 1;
>   sprintf(fname, "%s/.kvp_pool_%d", KVP_CONFIG_LOC, i);
> - fd = open(fname, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR |
> S_IROTH);
> + fd = open(fname, O_RDWR | O_CREAT, 0644 /* rw-r--r-- */);
> 
>   if (fd == -1)
>   return 1;
> --
> 1.7.11.7



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/3] tools/hv: Fix /var subdirectory

[KY Srinivasan]

> -Original Message-
> From: Tomas Hozza [mailto:tho...@redhat.com]
> Sent: Monday, November 12, 2012 3:55 AM
> To: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com; KY Srinivasan; b...@decadent.org.uk
> Cc: Tomas Hozza
> Subject: [PATCH 1/3] tools/hv: Fix /var subdirectory
> 
> Initial patch by Ben Hutchings 
> 
> We will install this in /usr, so it must use /var/lib for its state.
> Only programs installed under /opt should use /var/opt.
> 
> Signed-off-by: Tomas Hozza 
Acked-by:  K. Y. Srinivasan 


> ---
>  tools/hv/hv_kvp_daemon.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index 54ecb95..d80a612 100644
> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -98,7 +98,7 @@ static struct utsname uts_buf;
>   * The location of the interface configuration file.
>   */
> 
> -#define KVP_CONFIG_LOC   "/var/opt/"
> +#define KVP_CONFIG_LOC   "/var/lib/hyperv"
> 
>  #define MAX_FILE_NAME 100
>  #define ENTRIES_PER_BLOCK 50
> @@ -235,9 +235,9 @@ static int kvp_file_init(void)
>   int i;
>   int alloc_unit = sizeof(struct kvp_record) * ENTRIES_PER_BLOCK;
> 
> - if (access("/var/opt/hyperv", F_OK)) {
> - if (mkdir("/var/opt/hyperv", S_IRUSR | S_IWUSR | S_IROTH)) {
> - syslog(LOG_ERR, " Failed to create /var/opt/hyperv");
> + if (access(KVP_CONFIG_LOC, F_OK)) {
> + if (mkdir(KVP_CONFIG_LOC, S_IRUSR | S_IWUSR | S_IROTH)) {
> + syslog(LOG_ERR, " Failed to create %s",
> KVP_CONFIG_LOC);
>   exit(EXIT_FAILURE);
>   }
>   }
> @@ -246,7 +246,7 @@ static int kvp_file_init(void)
>   fname = kvp_file_info[i].fname;
>   records_read = 0;
>   num_blocks = 1;
> - sprintf(fname, "/var/opt/hyperv/.kvp_pool_%d", i);
> + sprintf(fname, "%s/.kvp_pool_%d", KVP_CONFIG_LOC, i);
>   fd = open(fname, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR |
> S_IROTH);
> 
>   if (fd == -1)
> @@ -1263,7 +1263,7 @@ static int kvp_set_ip_info(char *if_name, struct
> hv_kvp_ipaddr_value *new_val)
>*/
> 
>   snprintf(if_file, sizeof(if_file), "%s%s%s", KVP_CONFIG_LOC,
> - "hyperv/ifcfg-", if_name);
> + "/ifcfg-", if_name);
> 
>   file = fopen(if_file, "w");
> 
> --
> 1.7.11.7



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] Tools: hv: Fix for long file names from readdir

[KY Srinivasan]

> -Original Message-
> From: Tomas Hozza [mailto:tho...@redhat.com]
> Sent: Friday, November 09, 2012 7:47 AM
> To: o...@aepfle.de; KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; a...@canonical.com; jasow...@redhat.com;
> Tomas Hozza
> Subject: [PATCH] Tools: hv: Fix for long file names from readdir
> 
> kvp_get_if_name and kvp_mac_to_if_name copy strings into statically
> sized buffers which could be too small to store really long names.
> 
> Buffer sizes have been changed to PATH_MAX, include "limits.h" where
> PATH_MAX is defined was added and length checks ware added via snprintf.
> 
> Signed-off-by: Tomas Hozza 
Acked-by:  K. Y. Srinivasan 

> ---
>  tools/hv/hv_kvp_daemon.c | 26 +-
>  1 file changed, 9 insertions(+), 17 deletions(-)
> 
> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index 13c2a14..54ecb95 100644
> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -44,6 +44,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  /*
>   * KVP protocol: The user mode component first registers with the
> @@ -592,26 +593,22 @@ static char *kvp_get_if_name(char *guid)
>   DIR *dir;
>   struct dirent *entry;
>   FILE*file;
> - char*p, *q, *x;
> + char*p, *x;
>   char*if_name = NULL;
>   charbuf[256];
>   char *kvp_net_dir = "/sys/class/net/";
> - char dev_id[256];
> + char dev_id[PATH_MAX];
> 
>   dir = opendir(kvp_net_dir);
>   if (dir == NULL)
>   return NULL;
> 
> - snprintf(dev_id, sizeof(dev_id), "%s", kvp_net_dir);
> - q = dev_id + strlen(kvp_net_dir);
> -
>   while ((entry = readdir(dir)) != NULL) {
>   /*
>* Set the state for the next pass.
>*/
> - *q = '\0';
> - strcat(dev_id, entry->d_name);
> - strcat(dev_id, "/device/device_id");
> + snprintf(dev_id, sizeof(dev_id), "%s%s/device/device_id",
> kvp_net_dir,
> + entry->d_name);
> 
>   file = fopen(dev_id, "r");
>   if (file == NULL)
> @@ -684,28 +681,23 @@ static char *kvp_mac_to_if_name(char *mac)
>   DIR *dir;
>   struct dirent *entry;
>   FILE*file;
> - char*p, *q, *x;
> + char*p, *x;
>   char*if_name = NULL;
>   charbuf[256];
>   char *kvp_net_dir = "/sys/class/net/";
> - char dev_id[256];
> + char dev_id[PATH_MAX];
>   int i;
> 
>   dir = opendir(kvp_net_dir);
>   if (dir == NULL)
>   return NULL;
> 
> - snprintf(dev_id, sizeof(dev_id), kvp_net_dir);
> - q = dev_id + strlen(kvp_net_dir);
> -
>   while ((entry = readdir(dir)) != NULL) {
>   /*
>* Set the state for the next pass.
>*/
> - *q = '\0';
> -
> - strcat(dev_id, entry->d_name);
> - strcat(dev_id, "/address");
> + snprintf(dev_id, sizeof(dev_id), "%s%s/address", kvp_net_dir,
> +entry->d_name);
> 
>   file = fopen(dev_id, "r");
>   if (file == NULL)
> --
> 1.7.11.7



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/3] tools/hv: Fix /var subdirectory

[KY Srinivasan]

> -Original Message-
> From: gre...@linuxfoundation.org [mailto:gre...@linuxfoundation.org]
> Sent: Monday, November 26, 2012 4:12 PM
> To: KY Srinivasan
> Cc: Tomas Hozza; linux-kernel@vger.kernel.org; de...@linuxdriverproject.org;
> o...@aepfle.de; a...@canonical.com; jasow...@redhat.com;
> b...@decadent.org.uk
> Subject: Re: [PATCH 1/3] tools/hv: Fix /var subdirectory
> 
> On Mon, Nov 26, 2012 at 08:42:40PM +, KY Srinivasan wrote:
> >
> >
> > > -Original Message-
> > > From: Tomas Hozza [mailto:tho...@redhat.com]
> > > Sent: Monday, November 12, 2012 3:55 AM
> > > To: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> > > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> > > jasow...@redhat.com; KY Srinivasan; b...@decadent.org.uk
> > > Cc: Tomas Hozza
> > > Subject: [PATCH 1/3] tools/hv: Fix /var subdirectory
> > >
> > > Initial patch by Ben Hutchings 
> > >
> > > We will install this in /usr, so it must use /var/lib for its state.
> > > Only programs installed under /opt should use /var/opt.
> > >
> > > Signed-off-by: Tomas Hozza 
> > Acked-by:  K. Y. Srinivasan 
> 
> As I stated before, you need to rediff these, and resend them to me, I
> have no more hyperv patches in my queue to apply.

Will do. There were totally 3 patches that Tomas had sent that I acked today. 
Tomas, could you 
rebase the patches (if needed) and re-send them. 

K. Y
> 
> greg k-h
> 



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/3] tools/hv: Fix for long file names from readdir

[KY Srinivasan]

> -Original Message-
> From: Tomas Hozza [mailto:tho...@redhat.com]
> Sent: Tuesday, November 27, 2012 2:57 AM
> To: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com; KY Srinivasan; b...@decadent.org.uk
> Cc: Tomas Hozza
> Subject: [PATCH 1/3] tools/hv: Fix for long file names from readdir
> 
> kvp_get_if_name and kvp_mac_to_if_name copy strings into statically
> sized buffers which could be too small to store really long names.
> 
> Buffer sizes have been changed to PATH_MAX, include "limits.h" where
> PATH_MAX is defined was added and length checks ware added via snprintf.
> 
> Signed-off-by: Tomas Hozza 
Acked-by:  K. Y. Srinivasan 

> ---
>  tools/hv/hv_kvp_daemon.c | 26 +-
>  1 file changed, 9 insertions(+), 17 deletions(-)
> 
> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index d25a469..90f1f07 100644
> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -44,6 +44,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  /*
>   * KVP protocol: The user mode component first registers with the
> @@ -592,26 +593,22 @@ static char *kvp_get_if_name(char *guid)
>   DIR *dir;
>   struct dirent *entry;
>   FILE*file;
> - char*p, *q, *x;
> + char*p, *x;
>   char*if_name = NULL;
>   charbuf[256];
>   char *kvp_net_dir = "/sys/class/net/";
> - char dev_id[256];
> + char dev_id[PATH_MAX];
> 
>   dir = opendir(kvp_net_dir);
>   if (dir == NULL)
>   return NULL;
> 
> - snprintf(dev_id, sizeof(dev_id), "%s", kvp_net_dir);
> - q = dev_id + strlen(kvp_net_dir);
> -
>   while ((entry = readdir(dir)) != NULL) {
>   /*
>* Set the state for the next pass.
>*/
> - *q = '\0';
> - strcat(dev_id, entry->d_name);
> - strcat(dev_id, "/device/device_id");
> + snprintf(dev_id, sizeof(dev_id), "%s%s/device/device_id",
> kvp_net_dir,
> + entry->d_name);
> 
>   file = fopen(dev_id, "r");
>   if (file == NULL)
> @@ -684,28 +681,23 @@ static char *kvp_mac_to_if_name(char *mac)
>   DIR *dir;
>   struct dirent *entry;
>   FILE*file;
> - char*p, *q, *x;
> + char*p, *x;
>   char*if_name = NULL;
>   charbuf[256];
>   char *kvp_net_dir = "/sys/class/net/";
> - char dev_id[256];
> + char dev_id[PATH_MAX];
>   int i;
> 
>   dir = opendir(kvp_net_dir);
>   if (dir == NULL)
>   return NULL;
> 
> - snprintf(dev_id, sizeof(dev_id), kvp_net_dir);
> - q = dev_id + strlen(kvp_net_dir);
> -
>   while ((entry = readdir(dir)) != NULL) {
>   /*
>* Set the state for the next pass.
>*/
> - *q = '\0';
> -
> - strcat(dev_id, entry->d_name);
> - strcat(dev_id, "/address");
> + snprintf(dev_id, sizeof(dev_id), "%s%s/address", kvp_net_dir,
> +entry->d_name);
> 
>   file = fopen(dev_id, "r");
>   if (file == NULL)
> --
> 1.7.11.7
> 
> 



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 3/3] tools/hv: Fix permissions of created directory and files

[KY Srinivasan]

> -Original Message-
> From: Tomas Hozza [mailto:tho...@redhat.com]
> Sent: Tuesday, November 27, 2012 2:57 AM
> To: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com; KY Srinivasan; b...@decadent.org.uk
> Cc: Tomas Hozza
> Subject: [PATCH 3/3] tools/hv: Fix permissions of created directory and files
> 
> From: Ben Hutchings 
> 
> It's silly to create directories without execute permission, or to
> give permissions to 'other' but not the group-owner.
> 
> Write the permissions in octal and 'ls -l' format since these are much
> easier to read than the named macros.
> 
> Signed-off-by: Ben Hutchings 
> Signed-off-by: Tomas Hozza 
Acked-by:  K. Y. Srinivasan 

> ---
>  tools/hv/hv_kvp_daemon.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index e266251..7105c7b 100644
> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -236,7 +236,7 @@ static int kvp_file_init(void)
>   int alloc_unit = sizeof(struct kvp_record) * ENTRIES_PER_BLOCK;
> 
>   if (access(KVP_CONFIG_LOC, F_OK)) {
> - if (mkdir(KVP_CONFIG_LOC, S_IRUSR | S_IWUSR | S_IROTH)) {
> + if (mkdir(KVP_CONFIG_LOC, 0755 /* rwxr-xr-x */)) {
>   syslog(LOG_ERR, " Failed to create %s",
> KVP_CONFIG_LOC);
>   exit(EXIT_FAILURE);
>   }
> @@ -247,7 +247,7 @@ static int kvp_file_init(void)
>   records_read = 0;
>   num_blocks = 1;
>   sprintf(fname, "%s/.kvp_pool_%d", KVP_CONFIG_LOC, i);
> - fd = open(fname, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR |
> S_IROTH);
> + fd = open(fname, O_RDWR | O_CREAT, 0644 /* rw-r--r-- */);
> 
>   if (fd == -1)
>   return 1;
> --
> 1.7.11.7
> 
> 



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 2/3] tools/hv: Fix /var subdirectory

[KY Srinivasan]

> -Original Message-
> From: Tomas Hozza [mailto:tho...@redhat.com]
> Sent: Tuesday, November 27, 2012 2:57 AM
> To: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com; KY Srinivasan; b...@decadent.org.uk
> Cc: Tomas Hozza
> Subject: [PATCH 2/3] tools/hv: Fix /var subdirectory
> 
> Initial patch by Ben Hutchings 
> 
> We will install this in /usr, so it must use /var/lib for its state.
> Only programs installed under /opt should use /var/opt.
> 
> Signed-off-by: Tomas Hozza 
Acked-by:  K. Y. Srinivasan 

> ---
>  tools/hv/hv_kvp_daemon.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index 90f1f07..e266251 100644
> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -98,7 +98,7 @@ static struct utsname uts_buf;
>   * The location of the interface configuration file.
>   */
> 
> -#define KVP_CONFIG_LOC   "/var/opt/"
> +#define KVP_CONFIG_LOC   "/var/lib/hyperv"
> 
>  #define MAX_FILE_NAME 100
>  #define ENTRIES_PER_BLOCK 50
> @@ -235,9 +235,9 @@ static int kvp_file_init(void)
>   int i;
>   int alloc_unit = sizeof(struct kvp_record) * ENTRIES_PER_BLOCK;
> 
> - if (access("/var/opt/hyperv", F_OK)) {
> - if (mkdir("/var/opt/hyperv", S_IRUSR | S_IWUSR | S_IROTH)) {
> - syslog(LOG_ERR, " Failed to create /var/opt/hyperv");
> + if (access(KVP_CONFIG_LOC, F_OK)) {
> + if (mkdir(KVP_CONFIG_LOC, S_IRUSR | S_IWUSR | S_IROTH)) {
> + syslog(LOG_ERR, " Failed to create %s",
> KVP_CONFIG_LOC);
>   exit(EXIT_FAILURE);
>   }
>   }
> @@ -246,7 +246,7 @@ static int kvp_file_init(void)
>   fname = kvp_file_info[i].fname;
>   records_read = 0;
>   num_blocks = 1;
> - sprintf(fname, "/var/opt/hyperv/.kvp_pool_%d", i);
> + sprintf(fname, "%s/.kvp_pool_%d", KVP_CONFIG_LOC, i);
>   fd = open(fname, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR |
> S_IROTH);
> 
>   if (fd == -1)
> @@ -1263,7 +1263,7 @@ static int kvp_set_ip_info(char *if_name, struct
> hv_kvp_ipaddr_value *new_val)
>*/
> 
>   snprintf(if_file, sizeof(if_file), "%s%s%s", KVP_CONFIG_LOC,
> - "hyperv/ifcfg-", if_name);
> + "/ifcfg-", if_name);
> 
>   file = fopen(if_file, "w");
> 
> --
> 1.7.11.7
> 
> 



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: drivres/hv

[KY Srinivasan]

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Wednesday, November 28, 2012 5:43 PM
> To: KY Srinivasan
> Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org
> Subject: Re: drivres/hv
> 
> On Wed, Nov 28, 2012 at 12:17:30PM -0800, K. Y. Srinivasan wrote:
> >
> > Greg,
> 
> You are writing the most unhelpful Subject: lines lately, please be more
> descriptive in the future.

Sorry about that; I will be more descriptive.

> 
> > I recently discovered that the Hyper-V host allocates mmio
> > memory for some synthetic devices like the virtual framebuffer.
> > We are currently in the process of implementing this virtual
> > framebuffer device. As part of the offer message from the host
> > we are given the mmio region size that needs to be allocatted to
> > the device. I am told in the current implementation of the firmware,
> > this mmio resource shows up in the PCI space. What is the best way to
> > allocate this mmio space for this driver.
> 
> I don't understand, does the guest os think this really is mmio memory
> and the host just set up up?  Or is the memory being used to pass data
> back and forth?  Or something else?

>From what I understand, the host is allocating this memory and presenting
this to the guest. This memory is to be used for framebuffer. The guest is
expected to setup appropriate mappings to this region and use that as the 
framebuffer.

> 
> And what do you mean "firmware"?  That usually means UEFI/BIOS to me,
> not a hyperv host.

Hyper-V host presents the firmware to the guest (Hyper-V like KVM supports
full virtualization with selective para-virtualization - PV drivers). As part 
of the
BIOS presented to the guest this mmio region is allocated.

> 
> And finally, what does the guest os see as far as the PCI resource space
> shows it?  Shouldn't it just think it is a normal PCI device and access
> it properly that way?

This is the crux of the problem. Vmbus and other vmbus based devices are not
PCI  devices. I have implemented vmbus as an ACPI driver since vmbus is an ACPI
enumerated device. I have the size information of the mmio region that is to be 
allocated to the framebuffer device. I am looking at a way to allocate this mmio
region.

> 
> thanks,
> 
> greg k-h
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: drivres/hv

[KY Srinivasan]

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Wednesday, November 28, 2012 7:05 PM
> To: KY Srinivasan
> Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org
> Subject: Re: drivres/hv
> 
> On Wed, Nov 28, 2012 at 11:32:52PM +, KY Srinivasan wrote:
> > > > I recently discovered that the Hyper-V host allocates mmio
> > > > memory for some synthetic devices like the virtual framebuffer.
> > > > We are currently in the process of implementing this virtual
> > > > framebuffer device. As part of the offer message from the host
> > > > we are given the mmio region size that needs to be allocatted to
> > > > the device. I am told in the current implementation of the firmware,
> > > > this mmio resource shows up in the PCI space. What is the best way to
> > > > allocate this mmio space for this driver.
> > >
> > > I don't understand, does the guest os think this really is mmio memory
> > > and the host just set up up?  Or is the memory being used to pass data
> > > back and forth?  Or something else?
> >
> > From what I understand, the host is allocating this memory and presenting
> > this to the guest. This memory is to be used for framebuffer. The guest is
> > expected to setup appropriate mappings to this region and use that as the
> > framebuffer.
> 
> What type of framebuffer?  Linux supports a ton of different ones, have
> you looked at drivers/video/ to see if one of the ones there will "just
> work" with what hyperv is passing to the guest?

We are writing a diver for this device. The existing Linux drivers can (and do)
work for the emulated VGA device. What we are implementing is a synthetic
framebuffer driver that will communicate over the vmbus to the host.
 
> 
> If not, you'll have to write a new driver for this, look at the examples
> of other framebuffer drivers in that directory for examples.  Also, I
> bet the code there will answer your memory questions.
> 
> > > And what do you mean "firmware"?  That usually means UEFI/BIOS to me,
> > > not a hyperv host.
> >
> > Hyper-V host presents the firmware to the guest (Hyper-V like KVM supports
> > full virtualization with selective para-virtualization - PV drivers). As 
> > part of the
> > BIOS presented to the guest this mmio region is allocated.
> >
> > >
> > > And finally, what does the guest os see as far as the PCI resource space
> > > shows it?  Shouldn't it just think it is a normal PCI device and access
> > > it properly that way?
> >
> > This is the crux of the problem. Vmbus and other vmbus based devices are not
> > PCI  devices. I have implemented vmbus as an ACPI driver since vmbus is an
> ACPI
> > enumerated device. I have the size information of the mmio region that is 
> > to be
> > allocated to the framebuffer device. I am looking at a way to allocate this 
> > mmio
> > region.
> 
> Do you have any "hints" as to where it is, if it even is present, or
> anything else?  If so, use them and create a new framebuffer device with
> that information.

All I know is that it is present and its size is known. If I have a way of 
knowing
what range of mmio memory is unclaimed; I could grab that perhaps using
the request_mem_region() call, I know the size that is reserved for this device.
So, is there a way to query the system for the first unclaimed address in the 
mmio
range.

Thanks,

K. Y
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH -next] hv: hv_balloon: remove duplicated include from hv_balloon.c

[KY Srinivasan]

> -Original Message-
> From: Wei Yongjun [mailto:weiyj...@gmail.com]
> Sent: Wednesday, November 28, 2012 9:23 PM
> To: KY Srinivasan; Haiyang Zhang
> Cc: yongjun_...@trendmicro.com.cn; de...@linuxdriverproject.org; linux-
> ker...@vger.kernel.org
> Subject: [PATCH -next] hv: hv_balloon: remove duplicated include from
> hv_balloon.c
> 
> From: Wei Yongjun 
> 
> Remove duplicated include.
> 
> Signed-off-by: Wei Yongjun 
Acked-by:  K. Y. Srinivasan 

> ---
>  drivers/hv/hv_balloon.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> index f6c0011..0ce08c4 100644
> --- a/drivers/hv/hv_balloon.c
> +++ b/drivers/hv/hv_balloon.c
> @@ -29,7 +29,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
> 
>  #include 
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH -next] hv: hv_balloon: remove duplicated include from hv_balloon.c

[KY Srinivasan]

> -Original Message-
> From: Wei Yongjun [mailto:weiyj...@gmail.com]
> Sent: Wednesday, November 28, 2012 9:23 PM
> To: KY Srinivasan; Haiyang Zhang
> Cc: yongjun_...@trendmicro.com.cn; de...@linuxdriverproject.org; linux-
> ker...@vger.kernel.org
> Subject: [PATCH -next] hv: hv_balloon: remove duplicated include from
> hv_balloon.c
> 
> From: Wei Yongjun 
> 
> Remove duplicated include.
> 
> Signed-off-by: Wei Yongjun 
> ---
>  drivers/hv/hv_balloon.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> index f6c0011..0ce08c4 100644
> --- a/drivers/hv/hv_balloon.c
> +++ b/drivers/hv/hv_balloon.c
> @@ -29,7 +29,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
> 
>  #include 

Could you generate patches using git.

Regards,

K. Y
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH -next] hv: hv_balloon: remove duplicated include from hv_balloon.c

[KY Srinivasan]

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Thursday, November 29, 2012 12:31 PM
> To: KY Srinivasan
> Cc: Wei Yongjun; Haiyang Zhang; de...@linuxdriverproject.org;
> yongjun_...@trendmicro.com.cn; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH -next] hv: hv_balloon: remove duplicated include from
> hv_balloon.c
> 
> On Thu, Nov 29, 2012 at 05:09:42PM +, KY Srinivasan wrote:
> >
> >
> > > -Original Message-
> > > From: Wei Yongjun [mailto:weiyj...@gmail.com]
> > > Sent: Wednesday, November 28, 2012 9:23 PM
> > > To: KY Srinivasan; Haiyang Zhang
> > > Cc: yongjun_...@trendmicro.com.cn; de...@linuxdriverproject.org; linux-
> > > ker...@vger.kernel.org
> > > Subject: [PATCH -next] hv: hv_balloon: remove duplicated include from
> > > hv_balloon.c
> > >
> > > From: Wei Yongjun 
> > >
> > > Remove duplicated include.
> > >
> > > Signed-off-by: Wei Yongjun 
> > > ---
> > >  drivers/hv/hv_balloon.c | 1 -
> > >  1 file changed, 1 deletion(-)
> > >
> > > diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> > > index f6c0011..0ce08c4 100644
> > > --- a/drivers/hv/hv_balloon.c
> > > +++ b/drivers/hv/hv_balloon.c
> > > @@ -29,7 +29,6 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > -#include 
> > >  #include 
> > >
> > >  #include 
> >
> > Could you generate patches using git.
> 
> Why do you think this wasn't generated using git?

My mistake! 

K. Y
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/2] mm: Export split_page()

[KY Srinivasan]

> -Original Message-
> From: Michal Hocko [mailto:mho...@suse.cz]
> Sent: Monday, March 18, 2013 7:04 AM
> To: KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> a...@firstfloor.org; a...@linux-foundation.org; linux...@kvack.org;
> kamezawa.hiroy...@gmail.com; han...@cmpxchg.org; ying...@google.com
> Subject: Re: [PATCH 1/2] mm: Export split_page()
> 
> On Sat 16-03-13 14:42:04, K. Y. Srinivasan wrote:
> > The split_page() function will be very useful for balloon drivers. On 
> > Hyper-V,
> > it will be very efficient to use 2M allocations in the guest as this (a) 
> > makes
> > the ballooning protocol with the host that much more efficient and (b) 
> > moving
> > memory in 2M chunks minimizes fragmentation in the host. Export the
> split_page()
> > function to let the guest allocations be in 2M chunks while the host is 
> > free to
> > return this memory at arbitrary granularity.
> >
> > Signed-off-by: K. Y. Srinivasan 
> 
> I do not have any objections to exporting the symbol (at least we
> prevent drivers code from inventing their own split_page) but the
> Hyper-V specific description should go into Hyper-V patch IMO.
> 
> So for the export with a short note that the symbol will be used by
> Hyper-V

Will do.

K. Y
> Acked-by: Michal Hocko 
> 
> > ---
> >  mm/page_alloc.c |1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 6cacfee..7e0ead6 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -1404,6 +1404,7 @@ void split_page(struct page *page, unsigned int order)
> > for (i = 1; i < (1 << order); i++)
> > set_page_refcounted(page + i);
> >  }
> > +EXPORT_SYMBOL_GPL(split_page);
> >
> >  static int __isolate_free_page(struct page *page, unsigned int order)
> >  {
> > --
> > 1.7.4.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> 
> --
> Michal Hocko
> SUSE Labs
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 2/2] Drivers: hv: balloon: Support 2M page allocations for ballooning

[KY Srinivasan]

> -Original Message-
> From: Michal Hocko [mailto:mho...@suse.cz]
> Sent: Monday, March 18, 2013 6:53 AM
> To: KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> a...@firstfloor.org; a...@linux-foundation.org; linux...@kvack.org;
> kamezawa.hiroy...@gmail.com; han...@cmpxchg.org; ying...@google.com
> Subject: Re: [PATCH 2/2] Drivers: hv: balloon: Support 2M page allocations for
> ballooning
> 
> On Sat 16-03-13 14:42:05, K. Y. Srinivasan wrote:
> > While ballooning memory out of the guest, attempt 2M allocations first.
> > If 2M allocations fail, then go for 4K allocations. In cases where we
> > have performed 2M allocations, split this 2M page so that we can free this
> > page at 4K granularity (when the host returns the memory).
> 
> Maybe I am missing something but what is the advantage of 2M allocation
> when you split it up immediately so you are not using it as a huge page?

The Hyper-V ballooning protocol specifies the pages being ballooned as page 
ranges -
start_pfn: number_of_pfns. So, when the guest balloon is inflating and I am 
able to
allocate 2M pages, I will be able to represent 512 contiguous pages in one 64 
bit entry and this makes
the ballooning operation that much more efficient. The reason I split the page 
is that the host does not
guarantee that when it returns the memory to the guest, it will return in any 
particular granularity and so
I have to be able to free this memory in 4K granularity. This is the corner 
case that I will have to handle.

Regards,

K. Y
> 
> [...]
> --
> Michal Hocko
> SUSE Labs
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 2/2] Drivers: hv: balloon: Support 2M page allocations for ballooning

[KY Srinivasan]

> -Original Message-
> From: Michal Hocko [mailto:mho...@suse.cz]
> Sent: Monday, March 18, 2013 10:13 AM
> To: KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> a...@firstfloor.org; a...@linux-foundation.org; linux...@kvack.org;
> kamezawa.hiroy...@gmail.com; han...@cmpxchg.org; ying...@google.com
> Subject: Re: [PATCH 2/2] Drivers: hv: balloon: Support 2M page allocations for
> ballooning
> 
> On Mon 18-03-13 13:44:05, KY Srinivasan wrote:
> >
> >
> > > -Original Message-
> > > From: Michal Hocko [mailto:mho...@suse.cz]
> > > Sent: Monday, March 18, 2013 6:53 AM
> > > To: KY Srinivasan
> > > Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> > > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> > > a...@firstfloor.org; a...@linux-foundation.org; linux...@kvack.org;
> > > kamezawa.hiroy...@gmail.com; han...@cmpxchg.org;
> ying...@google.com
> > > Subject: Re: [PATCH 2/2] Drivers: hv: balloon: Support 2M page 
> > > allocations for
> > > ballooning
> > >
> > > On Sat 16-03-13 14:42:05, K. Y. Srinivasan wrote:
> > > > While ballooning memory out of the guest, attempt 2M allocations first.
> > > > If 2M allocations fail, then go for 4K allocations. In cases where we
> > > > have performed 2M allocations, split this 2M page so that we can free 
> > > > this
> > > > page at 4K granularity (when the host returns the memory).
> > >
> > > Maybe I am missing something but what is the advantage of 2M allocation
> > > when you split it up immediately so you are not using it as a huge page?
> >
> > The Hyper-V ballooning protocol specifies the pages being ballooned as
> > page ranges - start_pfn: number_of_pfns. So, when the guest balloon
> > is inflating and I am able to allocate 2M pages, I will be able to
> > represent 512 contiguous pages in one 64 bit entry and this makes the
> > ballooning operation that much more efficient. The reason I split the
> > page is that the host does not guarantee that when it returns the
> > memory to the guest, it will return in any particular granularity and
> > so I have to be able to free this memory in 4K granularity. This is
> > the corner case that I will have to handle.
> 
> Thanks for the clarification. I think this information would be valuable
> in the changelog.

Thanks Michal. I will resend the patches with the changes you have suggested. 
What is your
recommendation with regards which tree the mm patch needs to go through; the 
Hyper-V balloon
driver patch will go through Greg's tree.

Regards,

K. Y 
> --
> Michal Hocko
> SUSE Labs
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/1] Drivers: hv: Notify the host of permanent hot-add failures

[KY Srinivasan]

> -Original Message-
> From: K. Y. Srinivasan [mailto:k...@microsoft.com]
> Sent: Sunday, March 17, 2013 11:08 PM
> To: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com
> Cc: KY Srinivasan
> Subject: [PATCH 1/1] Drivers: hv: Notify the host of permanent hot-add 
> failures
> 
> If memory hot-add fails with the error -EEXIST, then this is a permanent
> failure. Notify the host this information, so the host will not attempt
> hot-add again. If the failure were a transient failure, host will attempt
> a hot-add after some delay.

Greg,

Please drop this patch; I am going to resend this as part of the earlier 
balloon driver patches I had sent.

Regards,

K. Y
> 
> Signed-off-by: K. Y. Srinivasan 
> ---
>  drivers/hv/hv_balloon.c |   17 +++--
>  1 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> index 71655b4..d2ed673 100644
> --- a/drivers/hv/hv_balloon.c
> +++ b/drivers/hv/hv_balloon.c
> @@ -583,6 +583,16 @@ static void hv_mem_hot_add(unsigned long start,
> unsigned long size,
> 
>   if (ret) {
>   pr_info("hot_add memory failed error is %d\n", ret);
> + if (ret == -EEXIST) {
> + /*
> +  * This error indicates that the error
> +  * is not a transient failure. This is the
> +  * case where the guest's physical address map
> +  * precludes hot adding memory. Stop all further
> +  * memory hot-add.
> +  */
> + do_hot_add = false;
> + }
>   has->ha_end_pfn -= HA_CHUNK;
>   has->covered_end_pfn -=  processed_pfn;
>   break;
> @@ -842,11 +852,14 @@ static void hot_add_req(struct work_struct *dummy)
>   rg_sz = region_size;
>   }
> 
> - resp.page_count = process_hot_add(pg_start, pfn_cnt,
> - rg_start, rg_sz);
> + if (do_hot_add)
> + resp.page_count = process_hot_add(pg_start, pfn_cnt,
> + rg_start, rg_sz);
>  #endif
>   if (resp.page_count > 0)
>   resp.result = 1;
> + else if (!do_hot_add)
> + resp.result = 1;
>   else
>   resp.result = 0;
> 
> --
> 1.7.4.1
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH V2 2/3] Drivers: hv: balloon: Support 2M page allocations for ballooning

[KY Srinivasan]

> -Original Message-
> From: Michal Hocko [mailto:mho...@suse.cz]
> Sent: Tuesday, March 19, 2013 10:46 AM
> To: KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> a...@firstfloor.org; a...@linux-foundation.org; linux...@kvack.org;
> kamezawa.hiroy...@gmail.com; han...@cmpxchg.org; ying...@google.com
> Subject: Re: [PATCH V2 2/3] Drivers: hv: balloon: Support 2M page allocations 
> for
> ballooning
> 
> On Mon 18-03-13 13:51:37, K. Y. Srinivasan wrote:
> > On Hyper-V it will be very efficient to use 2M allocations in the guest as 
> > this
> > makes the ballooning protocol with the host that much more efficient. 
> > Hyper-V
> > uses page ranges (start pfn : number of pages) to specify memory being moved
> > around and with 2M pages this encoding can be very efficient. However, when
> > memory is returned to the guest, the host does not guarantee any 
> > granularity.
> > To deal with this issue, split the page soon after a successful 2M 
> > allocation
> > so that this memory can potentially be freed as 4K pages.
> 
> How many pages are requested usually?

This depends entirely on how many pages the guest has, the pressure reported by 
the guest and
the overall memory demand as perceived by the host. On idling guests that have 
been configured with
several Giga bytes of memory, I have seen several Giga bytes being ballooned 
out of the guest as soon
as new VMs are started or pressure goes up in an existing VM. In these cases, 
if 2M allocations succeed,
the ballooning operation can be done very efficiently.
> 
> > If 2M allocations fail, we revert to 4K allocations.
> >
> > In this version of the patch, based on the feedback from Michal Hocko
> > , I have added some additional commentary to the patch
> > description.
> >
> > Signed-off-by: K. Y. Srinivasan 
> 
> I am not going to ack the patch because I am still not entirely
> convinced that big allocations are worth it. But that is up to you and
> hyper-V users.

As I said, Hyper-V has chosen 2M unit as the unit that will be most efficient 
in moving memory
around. All I am doing is making Linux participate in this protocol just as 
efficiently as Windows 
guests do.

Regards,

K. Y
> 
> > ---
> >  drivers/hv/hv_balloon.c |   18 --
> >  1 files changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> > index 2cf7d4e..71655b4 100644
> > --- a/drivers/hv/hv_balloon.c
> > +++ b/drivers/hv/hv_balloon.c
> > @@ -997,6 +997,14 @@ static int  alloc_balloon_pages(struct
> hv_dynmem_device *dm, int num_pages,
> >
> > dm->num_pages_ballooned += alloc_unit;
> >
> > +   /*
> > +* If we allocatted 2M pages; split them so we
> > +* can free them in any order we get.
> > +*/
> > +
> > +   if (alloc_unit != 1)
> > +   split_page(pg, get_order(alloc_unit << PAGE_SHIFT));
> > +
> > bl_resp->range_count++;
> > bl_resp->range_array[i].finfo.start_page =
> > page_to_pfn(pg);
> 
> I would suggest also using __GFP_NO_KSWAPD (or basically use
> GFP_TRANSHUGE for alloc_unit>0) for the allocation to be as least
> disruptive as possible.
> 
> > @@ -1023,9 +1031,10 @@ static void balloon_up(struct work_struct *dummy)
> >
> >
> > /*
> > -* Currently, we only support 4k allocations.
> > +* We will attempt 2M allocations. However, if we fail to
> > +* allocate 2M chunks, we will go back to 4k allocations.
> >  */
> > -   alloc_unit = 1;
> > +   alloc_unit = 512;
> >
> > while (!done) {
> > bl_resp = (struct dm_balloon_response *)send_buffer;
> > @@ -1041,6 +1050,11 @@ static void balloon_up(struct work_struct *dummy)
> > bl_resp, alloc_unit,
> >  &alloc_error);
> >
> 
> You should handle alloc_balloon_pages returns 0 && !alloc_error which
> happens when num_pages < alloc_unit.
> 
> > +   if ((alloc_error) && (alloc_unit != 1)) {
> > +   alloc_unit = 1;
> > +   continue;
> > +   }
> > +
> > if ((alloc_error) || (num_ballooned == num_pages)) {
> > bl_resp->more_pages = 0;
> > done = true;
> > --
> > 1.7.4.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> 
> --
> Michal Hocko
> SUSE Labs
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH V2 1/3] mm: Export split_page()

[KY Srinivasan]

> -Original Message-
> From: Michal Hocko [mailto:mho...@suse.cz]
> Sent: Tuesday, March 19, 2013 10:13 AM
> To: KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> a...@firstfloor.org; a...@linux-foundation.org; linux...@kvack.org;
> kamezawa.hiroy...@gmail.com; han...@cmpxchg.org; ying...@google.com
> Subject: Re: [PATCH V2 1/3] mm: Export split_page()
> 
> On Mon 18-03-13 13:51:36, K. Y. Srinivasan wrote:
> > This symbol would be used in the Hyper-V balloon driver to support 2M
> > allocations.
> >
> > In this version of the patch, based on feedback from Michal Hocko
> > , I have updated the patch description.
> 
> I guess this part is not necessary ;)
> 
> >
> > Signed-off-by: K. Y. Srinivasan 
> 
> Anyway
> Acked-by: Michal Hocko 

Greg,

Would you be taking this patch through your tree?

Regards,

K. Y
> 
> > ---
> >  mm/page_alloc.c |1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 6cacfee..7e0ead6 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -1404,6 +1404,7 @@ void split_page(struct page *page, unsigned int order)
> > for (i = 1; i < (1 << order); i++)
> > set_page_refcounted(page + i);
> >  }
> > +EXPORT_SYMBOL_GPL(split_page);
> >
> >  static int __isolate_free_page(struct page *page, unsigned int order)
> >  {
> > --
> > 1.7.4.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> 
> --
> Michal Hocko
> SUSE Labs
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: linux-next: manual merge of the akpm tree with the char-misc tree

[KY Srinivasan]

> -Original Message-
> From: Stephen Rothwell [mailto:s...@canb.auug.org.au]
> Sent: Wednesday, March 20, 2013 12:20 AM
> To: Andrew Morton
> Cc: linux-n...@vger.kernel.org; linux-kernel@vger.kernel.org; Haiyang Zhang; 
> KY
> Srinivasan; Greg KH; Arnd Bergmann
> Subject: linux-next: manual merge of the akpm tree with the char-misc tree
> 
> Hi Andrew,
> 
> Today's linux-next merge of the akpm tree got a conflict in
> include/linux/hyperv.h between commit 96dd86fa5881 ("Drivers: hv: Add a
> new driver to support host initiated backup") from the char-misc  tree
> and commit "drivers/video: add Hyper-V Synthetic Video Frame Buffer
> Driver" from the akpm tree.
> 
> I fixed it up (see below) and can carry the fix as necessary (no action
> is required).

Thanks Stephen,


K. Y
> 
> --
> Cheers,
> Stephen Rothwells...@canb.auug.org.au
> 
> diff --cc include/linux/hyperv.h
> index 95d0850,a460ee4..000
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@@ -1310,13 -1253,16 +1310,25 @@@ void vmbus_driver_unregister(struct
> hv_
>   }
> 
>   /*
>  + * VSS (Backup/Restore) GUID
>  + */
>  +#define HV_VSS_GUID \
>  +.guid = { \
>  +0x29, 0x2e, 0xfa, 0x35, 0x23, 0xea, 0x36, 0x42, \
>  +0x96, 0xae, 0x3a, 0x6e, 0xba, 0xcb, 0xa4,  0x40 \
>  +}
> ++
> ++/*
> +  * Synthetic Video GUID
> +  * {DA0A7802-E377-4aac-8E77-0558EB1073F8}
> +  */
> + #define HV_SYNTHVID_GUID \
> + .guid = { \
> + 0x02, 0x78, 0x0a, 0xda, 0x77, 0xe3, 0xac, 0x4a, \
> + 0x8e, 0x77, 0x05, 0x58, 0xeb, 0x10, 0x73, 0xf8 \
> + }
> +
> +
>   /*
>* Common header for Hyper-V ICs
>*/

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 0/3] X86: Deliver Hyper-V interrupts on a seperate IDT vector

[KY Srinivasan]

> -Original Message-
> From: K. Y. Srinivasan [mailto:k...@microsoft.com]
> Sent: Sunday, February 03, 2013 8:22 PM
> To: x...@kernel.org; gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com; t...@linutronix.de; h...@zytor.com;
> jbeul...@suse.com; b...@alien8.de
> Cc: KY Srinivasan
> Subject: [PATCH 0/3] X86: Deliver Hyper-V interrupts on a seperate IDT vector
> 
> This patch-set implements the functionality to deliver Hyper-V VMBUS 
> interrupts
> via
> a special IDT entry. Xen emulates Hyper-V and I have added code in this 
> patch-set
> to
> properly manage this emulation when Linux is running on Hyper-V.
> 
> K. Y. Srinivasan (2):
>   X86: Add a check to catch Xen emulation of Hyper-V
>   X86: Handle Hyper-V vmbus interrupts as special hypervisor interrupts
> 
> Olaf Hering (1):
>   x86: Hyper-V: register clocksource only if its advertised
> 
>  arch/x86/include/asm/irq_vectors.h |4 +-
>  arch/x86/include/asm/mshyperv.h|4 ++
>  arch/x86/kernel/cpu/mshyperv.c |   54
> +++-
>  arch/x86/kernel/entry_32.S |9 +-
>  arch/x86/kernel/entry_64.S |7 -
>  drivers/xen/events.c   |7 ++--
>  6 files changed, 77 insertions(+), 8 deletions(-)
> 
> --
> 1.7.4.1
> 
> 

Peter,

I have addressed all the comments I have received on these patches. Let me know 
if there is
something you want me to address before these patches can be checked in.

Regards,

K. Y

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: Regression, bisected: hyperv shutdown panics guest

[KY Srinivasan]

> -Original Message-
> From: Florian Westphal [mailto:f...@strlen.de]
> Sent: Thursday, February 07, 2013 5:54 AM
> To: Feng Hong
> Cc: ebied...@xmission.com; Kees Cook; linux-kernel@vger.kernel.org; KY
> Srinivasan; Haiyang Zhang
> Subject: Regression, bisected: hyperv shutdown panics guest
> 
> With 3.7, hyperv guest shutdown no longer works.
> Instead, guest kernel throws a bunch of "BUG: scheduling-while-atomic"
> errors and then dies.
> 
> reverting
> 
> commit 6c0c0d4d1080840eabb3d055d2fd8191c5fd
> Author: hongfeng 
> Date:   Thu Oct 4 17:12:25 2012 -0700
> poweroff: fix bug in orderly_poweroff()
> 
> fixes this problem.
> 
> Greping for users of orderly_poweroff() shows that hyperv isn't
> the only caller that invokes the function from irq context.
> 
> In fact, kdoc for orderly_poweroff says:
> 
> * This may be called from any context to trigger a system shutdown.
> * If the orderly shutdown fails, it will force an immediate shutdown.
> 
> Any suggestions on how to properly fix this?
> 
> Thanks,
> Florian
> 

A patch has been submitted to fix this problem. It is in Greg's queue:
Subject: Re: [PATCH 6/6] Drivers: hv: Execute shutdown in a thread context


Regards,

K. Y

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 6/6] Drivers: hv: Execute shutdown in a thread context

[KY Srinivasan]

> -Original Message-
> From: gre...@linuxfoundation.org [mailto:gre...@linuxfoundation.org]
> Sent: Thursday, January 24, 2013 12:18 PM
> To: KY Srinivasan
> Cc: Jiri Kosina; o...@aepfle.de; jasow...@redhat.com; linux-
> ker...@vger.kernel.org; james.bottom...@hansenpartnership.com;
> a...@canonical.com; de...@linuxdriverproject.org; da...@davemloft.net
> Subject: Re: [PATCH 6/6] Drivers: hv: Execute shutdown in a thread context
> 
> On Thu, Jan 24, 2013 at 05:06:27PM +, KY Srinivasan wrote:
> >
> >
> > > -Original Message-
> > > From: Jiri Kosina [mailto:jkos...@suse.cz]
> > > Sent: Thursday, January 24, 2013 5:10 AM
> > > To: KY Srinivasan
> > > Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> > > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> > > jasow...@redhat.com; da...@davemloft.net;
> > > james.bottom...@hansenpartnership.com
> > > Subject: Re: [PATCH 6/6] Drivers: hv: Execute shutdown in a thread context
> > >
> > > On Wed, 23 Jan 2013, K. Y. Srinivasan wrote:
> > >
> > > > Execute the shutdown code in a thread context. With recent changes made
> to
> > > the
> > > > shutdown code, shutdown code cannot be invoked from an interrupt
> context.
> > > >
> > > > Signed-off-by: K. Y. Srinivasan 
> > > > Reviewed-by: Haiyang Zhang 
> > > > ---
> > > >  drivers/hv/hv_util.c |   12 +++-
> > > >  1 files changed, 11 insertions(+), 1 deletions(-)
> > > >
> > > > diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
> > > > index 8b7868a..aceb67f 100644
> > > > --- a/drivers/hv/hv_util.c
> > > > +++ b/drivers/hv/hv_util.c
> > > > @@ -49,6 +49,16 @@ static struct hv_util_service util_kvp = {
> > > > .util_deinit = hv_kvp_deinit,
> > > >  };
> > > >
> > > > +static void perform_shutdown(struct work_struct *dummy)
> > > > +{
> > > > +   orderly_poweroff(true);
> > > > +}
> > >
> > > Is there any particular reason for this kind of crazy indentation?
> > I don't know how this extra tab crept through! Greg, if you want I can 
> > resend
> > this patch minus the extra tab. Let me know.
> 
> I'll edit it by hand, but someone owes me a beer for it...  :)

I just saw that you closed the tree. Did these patches go in?

Regards,

K. Y
> 
> greg k-h
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 6/6] Drivers: hv: Execute shutdown in a thread context

[KY Srinivasan]

> -Original Message-
> From: gre...@linuxfoundation.org [mailto:gre...@linuxfoundation.org]
> Sent: Friday, February 08, 2013 7:44 PM
> To: KY Srinivasan
> Cc: Jiri Kosina; o...@aepfle.de; jasow...@redhat.com; linux-
> ker...@vger.kernel.org; james.bottom...@hansenpartnership.com;
> a...@canonical.com; de...@linuxdriverproject.org; da...@davemloft.net
> Subject: Re: [PATCH 6/6] Drivers: hv: Execute shutdown in a thread context
> 
> On Sat, Feb 09, 2013 at 12:31:49AM +, KY Srinivasan wrote:
> >
> >
> > > -Original Message-
> > > From: gre...@linuxfoundation.org [mailto:gre...@linuxfoundation.org]
> > > Sent: Thursday, January 24, 2013 12:18 PM
> > > To: KY Srinivasan
> > > Cc: Jiri Kosina; o...@aepfle.de; jasow...@redhat.com; linux-
> > > ker...@vger.kernel.org; james.bottom...@hansenpartnership.com;
> > > a...@canonical.com; de...@linuxdriverproject.org; da...@davemloft.net
> > > Subject: Re: [PATCH 6/6] Drivers: hv: Execute shutdown in a thread context
> > >
> > > On Thu, Jan 24, 2013 at 05:06:27PM +, KY Srinivasan wrote:
> > > >
> > > >
> > > > > -Original Message-
> > > > > From: Jiri Kosina [mailto:jkos...@suse.cz]
> > > > > Sent: Thursday, January 24, 2013 5:10 AM
> > > > > To: KY Srinivasan
> > > > > Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> > > > > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> > > > > jasow...@redhat.com; da...@davemloft.net;
> > > > > james.bottom...@hansenpartnership.com
> > > > > Subject: Re: [PATCH 6/6] Drivers: hv: Execute shutdown in a thread
> context
> > > > >
> > > > > On Wed, 23 Jan 2013, K. Y. Srinivasan wrote:
> > > > >
> > > > > > Execute the shutdown code in a thread context. With recent changes
> made
> > > to
> > > > > the
> > > > > > shutdown code, shutdown code cannot be invoked from an interrupt
> > > context.
> > > > > >
> > > > > > Signed-off-by: K. Y. Srinivasan 
> > > > > > Reviewed-by: Haiyang Zhang 
> > > > > > ---
> > > > > >  drivers/hv/hv_util.c |   12 +++-
> > > > > >  1 files changed, 11 insertions(+), 1 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
> > > > > > index 8b7868a..aceb67f 100644
> > > > > > --- a/drivers/hv/hv_util.c
> > > > > > +++ b/drivers/hv/hv_util.c
> > > > > > @@ -49,6 +49,16 @@ static struct hv_util_service util_kvp = {
> > > > > > .util_deinit = hv_kvp_deinit,
> > > > > >  };
> > > > > >
> > > > > > +static void perform_shutdown(struct work_struct *dummy)
> > > > > > +{
> > > > > > +   orderly_poweroff(true);
> > > > > > +}
> > > > >
> > > > > Is there any particular reason for this kind of crazy indentation?
> > > > I don't know how this extra tab crept through! Greg, if you want I can
> resend
> > > > this patch minus the extra tab. Let me know.
> > >
> > > I'll edit it by hand, but someone owes me a beer for it...  :)
> >
> > I just saw that you closed the tree. Did these patches go in?
> 
> They should have, didn't you get an email saying they were in?  If not,
> check my git trees to verify.

They have gone in; thanks. I have been having mail problems and had not
received the check-in email.

Regards,

K. Y
 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/1] Drivers: hv: vmbus: Use the new infrastructure for delivering VMBUS interrupts

[KY Srinivasan]

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Wednesday, February 06, 2013 2:13 PM
> To: KY Srinivasan
> Cc: x...@kernel.org; linux-kernel@vger.kernel.org; 
> de...@linuxdriverproject.org;
> o...@aepfle.de; a...@canonical.com; jasow...@redhat.com;
> t...@linutronix.de; h...@zytor.com; jbeul...@suse.com; b...@alien8.de
> Subject: Re: [PATCH 1/1] Drivers: hv: vmbus: Use the new infrastructure for
> delivering VMBUS interrupts
> 
> On Wed, Feb 06, 2013 at 07:25:59AM -0800, K. Y. Srinivasan wrote:
> > Use the infrastructure for delivering VMBUS interrupts using a
> > special vector. With this patch, we can now properly handle
> > the VMBUS interrupts that can be delivered on any CPU. Also,
> > turn on interrupt load balancing as well.
> >
> > This patch requires the infrastructure that was implemented in the patch:
> > X86: Handle Hyper-V vmbus interrupts as special hypervisor interrupts
> >
> 
> Because of the dependancy, this should go through the x86 tree as well:
>   Acked-by: Greg Kroah-Hartman 
> 
> 

Peter,

Let me know if I need to make any adjustments to these patches. These set of
patches are key for Linux scalability on Hyper-V.

Regards,

K. Y

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/1] Drivers: hv: vmbus: Use the new infrastructure for delivering VMBUS interrupts

[KY Srinivasan]

> -Original Message-
> From: H. Peter Anvin [mailto:h...@zytor.com]
> Sent: Tuesday, February 12, 2013 7:31 PM
> To: Greg KH
> Cc: KY Srinivasan; x...@kernel.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com; t...@linutronix.de; jbeul...@suse.com; b...@alien8.de
> Subject: Re: [PATCH 1/1] Drivers: hv: vmbus: Use the new infrastructure for
> delivering VMBUS interrupts
> 
> On 02/06/2013 11:13 AM, Greg KH wrote:
> > On Wed, Feb 06, 2013 at 07:25:59AM -0800, K. Y. Srinivasan wrote:
> >> Use the infrastructure for delivering VMBUS interrupts using a
> >> special vector. With this patch, we can now properly handle
> >> the VMBUS interrupts that can be delivered on any CPU. Also,
> >> turn on interrupt load balancing as well.
> >>
> >> This patch requires the infrastructure that was implemented in the patch:
> >> X86: Handle Hyper-V vmbus interrupts as special hypervisor interrupts
> >>
> >
> > Because of the dependancy, this should go through the x86 tree as well:
> > Acked-by: Greg Kroah-Hartman 
> >
> 
> This does not apply to v3.8-rc7 + the set of three vmbus patches for
> x86.  I take it there are additional dependencies -- K.Y., what do I
> need here?

Do none of the earlier patches apply or just this patch. If none of them
apply, I could rebase all of them and resend them. Please let me know.

Regards,

K. Y
> 
>   -hpa
> 
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/1] Drivers: hv: vmbus: Use the new infrastructure for delivering VMBUS interrupts

[KY Srinivasan]

> -Original Message-
> From: H. Peter Anvin [mailto:h...@zytor.com]
> Sent: Tuesday, February 12, 2013 7:46 PM
> To: KY Srinivasan
> Cc: Greg KH; x...@kernel.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com; t...@linutronix.de; jbeul...@suse.com; b...@alien8.de
> Subject: Re: [PATCH 1/1] Drivers: hv: vmbus: Use the new infrastructure for
> delivering VMBUS interrupts
> 
> On 02/12/2013 04:43 PM, KY Srinivasan wrote:
> >>
> >> This does not apply to v3.8-rc7 + the set of three vmbus patches for
> >> x86.  I take it there are additional dependencies -- K.Y., what do I
> >> need here?
> >
> > Do none of the earlier patches apply or just this patch. If none of them
> > apply, I could rebase all of them and resend them. Please let me know.
> >
> 
> The earlier (set of 3) patches apply just fine.

Apply them and I will rebase this patch and send it out.

Regards,

K. Y 
> 
>   -hpa
> 
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [tip:x86/hyperv] x86, hyperv: HYPERV depends on X86_LOCAL_APIC

[KY Srinivasan]

> -Original Message-
> From: tip tree robot [mailto:tip...@zytor.com] On Behalf Of tip-bot for H. 
> Peter
> Anvin
> Sent: Tuesday, February 12, 2013 8:59 PM
> To: linux-tip-comm...@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org; h...@zytor.com; mi...@kernel.org; KY
> Srinivasan; t...@linutronix.de; h...@linux.intel.com
> Subject: [tip:x86/hyperv] x86, hyperv: HYPERV depends on X86_LOCAL_APIC
> 
> Commit-ID:  cb20e5f2c8d6ba7440a32f4d70c0755bceb36e78
> Gitweb:
> http://git.kernel.org/tip/cb20e5f2c8d6ba7440a32f4d70c0755bceb36e78
> Author: H. Peter Anvin 
> AuthorDate: Tue, 12 Feb 2013 17:46:23 -0800
> Committer:  H. Peter Anvin 
> CommitDate: Tue, 12 Feb 2013 17:46:23 -0800
> 
> x86, hyperv: HYPERV depends on X86_LOCAL_APIC
> 
> In order to compile in the special Hyper-V interrupt vector, we need
> infrastructure in arch/x86/apic/apic.c.  At least for now, simply
> require CONFIG_X86_LOCAL_APIC in order to enable CONFIG_HYPERV.
> 
> Link: http://lkml.kernel.org/r/tip-
> bc2b0331e077f576369a2b6c75d15ed4de4ef...@git.kernel.org
> Cc: K. Y. Srinivasan 
> Signed-off-by: H. Peter Anvin 
> ---
>  drivers/hv/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
> index b38ef6d..64630f1 100644
> --- a/drivers/hv/Kconfig
> +++ b/drivers/hv/Kconfig
> @@ -2,7 +2,7 @@ menu "Microsoft Hyper-V guest support"
> 
>  config HYPERV
>   tristate "Microsoft Hyper-V client drivers"
> - depends on X86 && ACPI && PCI
> + depends on X86 && ACPI && PCI && X86_LOCAL_APIC
>   help
> Select this option to run Linux as a Hyper-V client operating
> system.
> 
Thank you.

K. Y
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�&j:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

RE: scanning for LUNs

[KY Srinivasan]

> -Original Message-
> From: James Bottomley [mailto:jbottom...@parallels.com]
> Sent: Thursday, April 04, 2013 11:15 AM
> To: KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; oher...@suse.com; h...@infradead.org; linux-
> s...@vger.kernel.org
> Subject: Re: scanning for LUNs
> 
> On Thu, 2013-04-04 at 08:12 -0700, K. Y. Srinivasan wrote:
> > Here is the code snippet for scanning LUNS (drivers/scsi/scsi_scan.c in 
> > function
> > __scsi_scan_target()):
> >
> > /*
> >  * Scan LUN 0, if there is some response, scan further. Ideally, we
> >  * would not configure LUN 0 until all LUNs are scanned.
> >  */
> > res = scsi_probe_and_add_lun(starget, 0, &bflags, NULL, rescan, 
> > NULL);
> > if (res == SCSI_SCAN_LUN_PRESENT || res ==
> SCSI_SCAN_TARGET_PRESENT) {
> > if (scsi_report_lun_scan(starget, bflags, rescan) != 0)
> >
> >
> > So, if we don't get a response while scanning LUN0, we will not use
> > scsi_report_lun_scan().
> > On Hyper-V, the scsi emulation on the host does not treat LUN0 as
> > anything special and we
> > could have situations where the only device under a scsi controller is
> > at a location other than 0
> > or 1. In this case the standard LUN scanning code in Linux fails to
> > detect this device. Is this
> > behaviour expected? Why is LUN0 treated differently here. Looking at
> > the scsi spec, I am not sure
> > if this is what is specified. Any help/guidance will be greatly
> > appreciated.
> 
> Why don't you describe the problem.  We can't scan randomly a bunch of
> LUNs hoping for a response (the space is 10^19).  SAM thinks you use
> LUNW for this, but that's not well supported.  We can't annoy USB
> devices by probing with REPORT LUNS, so conventionally most arrays
> return something for LUN0 even if they don't actually have one (That's
> what the peripheral qualifier codes are supposed to be about).  We
> translate PQ1 and PQ2 to SCSI_SCAN_TARGET_PRESENT, which means no LUN,
> but there is a target to scan here.
> 
> If you're sending back an error to an INQUIRY to LUN0, then you're out
> of spec.  The SCSI standards say:
> 
> SPC3 6.4.1: In response to an INQUIRY command received by an
> incorrect logical unit, the SCSI target device shall return the
> INQUIRY data with the peripheral qualifier set to the value
> defined in 6.4.2. The INQUIRY command shall return CHECK
> CONDITION status only when the device server is unable to return
> the requested INQUIRY data

Thanks James. I will further investigate the issue on our platform.

Regards,

K. Y
> 
> James
> 
> 
> James
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] x86: Hyper-V: register clocksource only if its advertised

[KY Srinivasan]

> -Original Message-
> From: Olaf Hering [mailto:o...@aepfle.de]
> Sent: Tuesday, January 29, 2013 4:27 AM
> To: Jan Beulich
> Cc: Stefano Stabellini; KY Srinivasan; Greg KH; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] x86: Hyper-V: register clocksource only if its advertised
> 
> On Tue, Jan 29, Jan Beulich wrote:
> 
> > >>> On 28.01.13 at 18:44, Stefano Stabellini 
> > >>> 
> wrote:
> > > I think that Olaf made his point very clear: a feature A should only be
> > > enabled if the corresponding flag A is set.
> > > In fact it seems to me that this patch is correct on its own merits,
> > > regardless of Xen does or does not.
> > >
> > > The Xen tools might or might not know whether a guest is going to be
> > > Linux, Windows, FreeBSD or whatever else people use nowadays.  Setting
> > > viridian=1 is the safe choice, given that it shouldn't create any
> > > issues: after all guests are supposed to check for feature flags before
> > > using them.
> > >
> > > If Xen is going to implement "Partition Reference Counter", it is also
> > > going to set the corresponding flag, so the guest OS (Windows, Linux,
> > > my pet OS) can check whether the feature is available and decide whether
> > > it wants to use it.
> >
> > While I agree in general, the specific case of the callback vector
> > seems a little more difficult: As KY says, there's no feature flag
> > for this (or perhaps more precisely for it being deliverable across
> > all CPUs), and hence there's both the problem of detection and
> > the problem of disambiguation (as otherwise both the Hyper-V
> > code and the Xen code in Linux could be trying to use the same
> > vector).
> 
> This is true, but belongs to that other thread about the interrupt
> vector. I agree that the detection in ms_hyperv_platform() should be
> extended, with a DMI check for example.
I want to explore extending ms_hyperv_platform() to detect if Hyper-V is being
emulated. Since Hyper-V is never going to emulate Xen, in my view this would be 
a
better option. 

 
> 
> The patch which started this thread is still valid because it enables
> feature B only if the featurebit for B is enabled.

Why would we need this if we have some other way of detecting that Hyper-V is 
being
emulated. Furthermore, I am not sure I understand the logic here. The fact that 
the hypervisor
supports PARTITION_REFERENCE_COUNTER does not necessarily mean that we should 
register
a clocksource based on that reference counter. It is true that that is the case 
on Hyper-V today.
However, on other hypervisors emulating Hyper-V, other standard clocksources 
maybe more
appropriate.

I agree with you that your patch is valid for making the Hyper-V code more 
robust but not for dealing
with general issue of Xen emulating Hyper-V. I will Ack your patch. 

Regards,

K. Y 
> 
> Olaf
> 

RE: [PATCH] x86: Hyper-V: register clocksource only if its advertised

[KY Srinivasan]

> -Original Message-
> From: Stefano Stabellini [mailto:stefano.stabell...@eu.citrix.com]
> Sent: Tuesday, January 29, 2013 10:31 AM
> To: KY Srinivasan
> Cc: Olaf Hering; Jan Beulich; Stefano Stabellini; Greg KH; linux-
> ker...@vger.kernel.org
> Subject: RE: [PATCH] x86: Hyper-V: register clocksource only if its advertised
> 
> On Tue, 29 Jan 2013, KY Srinivasan wrote:
> > > The patch which started this thread is still valid because it enables
> > > feature B only if the featurebit for B is enabled.
> >
> > Why would we need this if we have some other way of detecting that Hyper-V
> is being
> > emulated.
> 
> I don't think that Hyper-V being emulated or not matters here.
> What matters is whether the feature is available: only if it is then it
> should be used.
> 
> 
> > Furthermore, I am not sure I understand the logic here. The fact that the
> hypervisor
> > supports PARTITION_REFERENCE_COUNTER does not necessarily mean that we
> should register
> > a clocksource based on that reference counter. It is true that that is the 
> > case on
> Hyper-V today.
> > However, on other hypervisors emulating Hyper-V, other standard
> clocksources maybe more
> > appropriate.
> 
> The point is that if a feature is NOT supported, then it should NOT be
> used. Only if it is supported, then Linux might consider using it. And
> of course can decide not to use it.
> 
> 
> > I agree with you that your patch is valid for making the Hyper-V code more
> robust but not for dealing
> > with general issue of Xen emulating Hyper-V.
> 
> What "general issue" would that be?

Not all Hyper-V specific features are advertised via a feature bit. For 
instance, we need to reserve
an IDT entry to have VMBUS interrupts delivered on a per-CPU basis and this 
reservation is done
after confirming that we are running on Hyper-V. With Xen emulating Hyper-V, we 
have an issue.
Since Hyper-V will never emulate Xen, I am going to add a check in the Hyper-V 
detection code to
deal with this issue.

Regards,

K. Y
 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 2/3] X86: Add a check to catch Xen emulation of Hyper-V

[KY Srinivasan]

> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: Wednesday, January 30, 2013 3:53 AM
> To: KY Srinivasan
> Cc: o...@aepfle.de; b...@alien8.de; a...@canonical.com; x...@kernel.org;
> t...@linutronix.de; de...@linuxdriverproject.org; gre...@linuxfoundation.org;
> jasow...@redhat.com; linux-kernel@vger.kernel.org; h...@zytor.com
> Subject: Re: [PATCH 2/3] X86: Add a check to catch Xen emulation of Hyper-V
> 
> >>> On 30.01.13 at 01:51, "K. Y. Srinivasan"  wrote:
> > Xen emulates Hyper-V to host enlightened Windows. Looks like this
> > emulation may be turned on by default even for Linux guests. Check and
> > fail Hyper-V detection if we are on Xen.
> >
> > Signed-off-by: K. Y. Srinivasan 
> > ---
> >  arch/x86/kernel/cpu/mshyperv.c |7 +++
> >  1 files changed, 7 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/x86/kernel/cpu/mshyperv.c
> b/arch/x86/kernel/cpu/mshyperv.c
> > index 646d192..4dab317 100644
> > --- a/arch/x86/kernel/cpu/mshyperv.c
> > +++ b/arch/x86/kernel/cpu/mshyperv.c
> > @@ -30,6 +30,13 @@ static bool __init ms_hyperv_platform(void)
> > if (!boot_cpu_has(X86_FEATURE_HYPERVISOR))
> > return false;
> >
> > +   /*
> > +* Xen emulates Hyper-V to support enlightened Windows.
> > +* Check to see first if we are on a Xen Hypervisor.
> > +*/
> > +   if (xen_cpuid_base())
> > +   return false;
> > +
> > cpuid(HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS,
> >   &eax, &hyp_signature[0], &hyp_signature[1], &hyp_signature[2]);
> 
> I'm not convinced that's the right approach - any hypervisor
> could do similar emulation, and hence you either want to make
> sure you run on Hyper-V (by excluding all others), or you
> tolerate using the emulation (which may require syncing up with
> the other guest implementations so that shared resources don't
> get used by two parties).
> 
> I also wonder whether using the Hyper-V emulation (where
> useful, there might not be anything right now, but this may
> change going forward) when no Xen support is configured
> wouldn't be better than not using anything...

Jan,

Presumably, Hyper-V emulation is only to run enlightened Windows. The issue with
Xen is not that it emulates Hyper-V, but this emulation is turned on while 
running Linux.
That is the reason I chose to check for Xen. Would you prefer a DMI check for 
the Hyper-V
platform. 

Regards,

K. Y


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 2/3] X86: Add a check to catch Xen emulation of Hyper-V

[KY Srinivasan]

> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: Thursday, January 31, 2013 2:38 AM
> To: KY Srinivasan
> Cc: o...@aepfle.de; b...@alien8.de; a...@canonical.com; x...@kernel.org;
> t...@linutronix.de; de...@linuxdriverproject.org; gre...@linuxfoundation.org;
> jasow...@redhat.com; linux-kernel@vger.kernel.org; h...@zytor.com
> Subject: RE: [PATCH 2/3] X86: Add a check to catch Xen emulation of Hyper-V
> 
> >>> On 30.01.13 at 19:12, KY Srinivasan  wrote:
> > Presumably, Hyper-V emulation is only to run enlightened Windows. The issue
> > with
> > Xen is not that it emulates Hyper-V, but this emulation is turned on while
> > running Linux.
> > That is the reason I chose to check for Xen. Would you prefer a DMI check
> > for the Hyper-V
> > platform.
> 
> I consider DMI checks to be too fragile here - in particular with
> the eventual passing through of host DMI attributes to guests,
> this sets you up for mistakes. Instead, I would envision you
> scanning the whole CPUID range "reserved" for virtualization
> (starting at 0x4000) and see whether you get back
> anything other than the Hyper-V identification (much like the
> way xen_cpuid_base() scans for the Xen range). Of course
> that's under the premise that you're right in assuming Hyper-V
> would never emulate any other hypervisor's interface.

Agreed; I will make the appropriate changes as you have recommended.

Regards,

K. Y


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 3/3] X86: Handle Hyper-V vmbus interrupts as special hypervisor interrupts

[KY Srinivasan]

> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: Wednesday, January 30, 2013 3:59 AM
> To: KY Srinivasan
> Cc: o...@aepfle.de; b...@alien8.de; a...@canonical.com; x...@kernel.org;
> t...@linutronix.de; de...@linuxdriverproject.org; gre...@linuxfoundation.org;
> jasow...@redhat.com; linux-kernel@vger.kernel.org; h...@zytor.com
> Subject: Re: [PATCH 3/3] X86: Handle Hyper-V vmbus interrupts as special
> hypervisor interrupts
> 
> >>> On 30.01.13 at 01:51, "K. Y. Srinivasan"  wrote:
> > --- a/arch/x86/kernel/cpu/mshyperv.c
> > +++ b/arch/x86/kernel/cpu/mshyperv.c
> > @@ -14,10 +14,15 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> > +#include 
> > +#include 
> >
> >  struct ms_hyperv_info ms_hyperv;
> >  EXPORT_SYMBOL_GPL(ms_hyperv);
> > @@ -77,6 +82,12 @@ static void __init ms_hyperv_init_platform(void)
> >
> > if (ms_hyperv.features & HV_X64_MSR_TIME_REF_COUNT_AVAILABLE)
> > clocksource_register_hz(&hyperv_cs, NSEC_PER_SEC/100);
> > +#if IS_ENABLED(CONFIG_HYPERV)
> > +   /*
> > +* Setup the IDT for hypervisor callback.
> > +*/
> > +   alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR,
> hyperv_callback_vector);
> > +#endif
> >  }
> >
> >  const __refconst struct hypervisor_x86 x86_hyper_ms_hyperv = {
> > @@ -85,3 +96,30 @@ const __refconst struct hypervisor_x86
> x86_hyper_ms_hyperv = {
> > .init_platform  = ms_hyperv_init_platform,
> >  };
> >  EXPORT_SYMBOL(x86_hyper_ms_hyperv);
> > +
> > +static int vmbus_irq = -1;
> > +static irq_handler_t vmbus_isr;
> > +
> > +void hv_register_vmbus_handler(int irq, irq_handler_t handler)
> > +{
> > +   vmbus_irq = irq;
> > +   vmbus_isr = handler;
> > +}
> > +EXPORT_SYMBOL_GPL(hv_register_vmbus_handler);
> > +
> > +void hyperv_vector_handler(struct pt_regs *regs)
> > +{
> > +   struct pt_regs *old_regs = set_irq_regs(regs);
> > +   struct irq_desc *desc;
> > +
> > +   irq_enter();
> > +   exit_idle();
> > +
> > +   desc = irq_to_desc(vmbus_irq);
> > +
> > +   if (desc)
> > +   generic_handle_irq_desc(vmbus_irq, desc);
> > +
> > +   irq_exit();
> > +   set_irq_regs(old_regs);
> > +}
> 
> This function appears to be dead code when !CONFIG_HYPERV,
> because ...

I will make the necessary adjustments to deal with this.

> 
> > --- a/arch/x86/kernel/entry_32.S
> > +++ b/arch/x86/kernel/entry_32.S
> > @@ -1046,11 +1046,18 @@ ENTRY(xen_failsafe_callback)
> > _ASM_EXTABLE(4b,9b)
> >  ENDPROC(xen_failsafe_callback)
> >
> > -BUILD_INTERRUPT3(xen_hvm_callback_vector,
> XEN_HVM_EVTCHN_CALLBACK,
> > +BUILD_INTERRUPT3(xen_hvm_callback_vector,
> HYPERVISOR_CALLBACK_VECTOR,
> > xen_evtchn_do_upcall)
> >
> >  #endif /* CONFIG_XEN */
> >
> > +#if IS_ENABLED(CONFIG_HYPERV)
> > +
> > +BUILD_INTERRUPT3(hyperv_callback_vector,
> HYPERVISOR_CALLBACK_VECTOR,
> > +   hyperv_vector_handler)
> > +
> > +#endif /* CONFIG_HYPERV */
> > +
> >  #ifdef CONFIG_FUNCTION_TRACER
> >  #ifdef CONFIG_DYNAMIC_FTRACE
> >
> 
> ... the consumers here and below are conditional.
> 
> I also wonder why arch/x86/kernel/cpu/mshyperv.c is being built
> at all when !CONFIG_HYPERV (which would eliminate the need
> for the conditional the patch adds to ms_hyperv_init_platform()).

Linux is usable on Hyper-V in full emulation mode with the Hyper-V specific
clocksource plugged in. The current code permits that.

K. Y 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 2/3] X86: Add a check to catch Xen emulation of Hyper-V

[KY Srinivasan]

> -Original Message-
> From: devel [mailto:devel-boun...@linuxdriverproject.org] On Behalf Of KY
> Srinivasan
> Sent: Thursday, January 31, 2013 9:46 AM
> To: Jan Beulich
> Cc: o...@aepfle.de; gre...@linuxfoundation.org; jasow...@redhat.com;
> x...@kernel.org; linux-kernel@vger.kernel.org; b...@alien8.de; h...@zytor.com;
> a...@canonical.com; de...@linuxdriverproject.org; t...@linutronix.de
> Subject: RE: [PATCH 2/3] X86: Add a check to catch Xen emulation of Hyper-V
> 
> 
> 
> > -Original Message-
> > From: Jan Beulich [mailto:jbeul...@suse.com]
> > Sent: Thursday, January 31, 2013 2:38 AM
> > To: KY Srinivasan
> > Cc: o...@aepfle.de; b...@alien8.de; a...@canonical.com; x...@kernel.org;
> > t...@linutronix.de; de...@linuxdriverproject.org;
> gre...@linuxfoundation.org;
> > jasow...@redhat.com; linux-kernel@vger.kernel.org; h...@zytor.com
> > Subject: RE: [PATCH 2/3] X86: Add a check to catch Xen emulation of Hyper-V
> >
> > >>> On 30.01.13 at 19:12, KY Srinivasan  wrote:
> > > Presumably, Hyper-V emulation is only to run enlightened Windows. The
> issue
> > > with
> > > Xen is not that it emulates Hyper-V, but this emulation is turned on while
> > > running Linux.
> > > That is the reason I chose to check for Xen. Would you prefer a DMI check
> > > for the Hyper-V
> > > platform.
> >
> > I consider DMI checks to be too fragile here - in particular with
> > the eventual passing through of host DMI attributes to guests,
> > this sets you up for mistakes. Instead, I would envision you
> > scanning the whole CPUID range "reserved" for virtualization
> > (starting at 0x4000) and see whether you get back
> > anything other than the Hyper-V identification (much like the
> > way xen_cpuid_base() scans for the Xen range). Of course
> > that's under the premise that you're right in assuming Hyper-V
> > would never emulate any other hypervisor's interface.
> 
> Agreed; I will make the appropriate changes as you have recommended.

Jan,

Are there any published standards in terms of how the CPUID space should be 
populated in the range from 0x4000 to 0x4001. Specifically, unless the 
standard mandates that all ranges unused by a given hypervisor would return a 
known value, how can this code be used to detect the presence of an unknown 
hypervisor. Hyper-V is going to return the Hyper-V string at 0x4000. So, I 
was planning to scan starting at 0x4100. Clearly, I can check for a 
specific hypervisor that I know causes a problem for Hyper-V (as I have 
currently done by checking for Xen). How can I check for the presence of yet to 
be created Hypervisors that may emulate Hyper-V by scanning the CPUID space. I 
am almost tempted to say that Xen is the special case and the patch I have 
submitted addresses that. If a new (or existing hypervisor) plans to do what 
Xen is doing, perhaps we can dissuade them from doing that or we can fix that 
within the general framework we have here.

Regards,

K. Y
> ___
> devel mailing list
> de...@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 2/3] X86: Add a check to catch Xen emulation of Hyper-V

[KY Srinivasan]

> -Original Message-
> From: Stefano Stabellini [mailto:stefano.stabell...@eu.citrix.com]
> Sent: Friday, February 01, 2013 8:20 AM
> To: H. Peter Anvin
> Cc: Jan Beulich; KY Srinivasan; o...@aepfle.de; b...@alien8.de;
> a...@canonical.com; x...@kernel.org; t...@linutronix.de;
> de...@linuxdriverproject.org; gre...@linuxfoundation.org;
> jasow...@redhat.com; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 2/3] X86: Add a check to catch Xen emulation of Hyper-V
> 
> On Thu, 31 Jan 2013, H. Peter Anvin wrote:
> > On 01/30/2013 12:53 AM, Jan Beulich wrote:
> > >
> > > I'm not convinced that's the right approach - any hypervisor
> > > could do similar emulation, and hence you either want to make
> > > sure you run on Hyper-V (by excluding all others), or you
> > > tolerate using the emulation (which may require syncing up with
> > > the other guest implementations so that shared resources don't
> > > get used by two parties).
> > >
> > > I also wonder whether using the Hyper-V emulation (where
> > > useful, there might not be anything right now, but this may
> > > change going forward) when no Xen support is configured
> > > wouldn't be better than not using anything...
> > >
> >
> > I'm confused about "the right approach" here is.  As far as I
> > understand, this only can affect a Xen guest where HyperV guest support
> > is enabled but not Xen support, and only because Xen emulates HyperV but
> > does so incorrectly.
> >
> > This is a Xen bug, and as such it makes sense to reject Xen
> > specifically.  If another hypervisor emulates HyperV and does so
> > correctly there seems to be no reason to reject it.
> 
> I don't think so.
> 
> AFAIK originally there were features exported as flags and Xen doesn't
> turn on the flags that correspond to features that are not implemented.
> The problem here is that Hyper-V is about to introduce a feature without
> a flag that is not implemented by Xen (see "X86: Deliver Hyper-V
> interrupts on a separate IDT vector").
> K.Y. please confirm if I got this right.

I am not sure I can agree with you here. There are two discriminating factors
here: (a) Hypervisor check and (b) Feature check. Not every feature of the
hypervisor can be surfaced as feature bit and furthermore, just because a 
feature
bit is turned on, it does not necessarily mean that the feature is to be used. 
For instance,
let us say that Windows guests begin to use the "partition counter" and Xen 
chooses
to implement that to better support Windows. This does not mean that while 
hosting
Linux on Xen, you want to plug in a clock source based on the emulated
"partition counter". This is what would happen in the code we have today.

Other Hypervisors emulating Hyper-V do not have this problem and Xen would not 
either
if the emulation bit is selectively turned on (only while running Windows) or 
if Xen were allowed
to check first ahead of Hyper-V (unconditionally) in the hypervisor detection 
code. As Peter pointed out, we 
have this problem because of the unique situation with Xen.

In any event, I am not going to further argue this issue; this last round of 
patches I sent out,
fixes the issue for Xen. Jan wants me to make this check more general. While I 
don't think
we need to do that, I will see if I can do it. I am checking to see if MSFT 
guarantees that Hyper-V
would initialize the unused CPUID space to 0. If this is the case, I will 
implement the check
Jan has suggested; if not, we have to live with the Xen specific check that I 
currently have.

> 
> If I were the Microsoft engineer implementing this feature, no matter
> what Xen does or does not, I would also make sure that there is a
> corresponding flag for it, because in my experience they avoid future
> headaches.
> I wonder what happens if you run Linux with Hyper-V support on an old
> Hyper-V host that doesn't support vector injection.
> 

To answer your specific question, this feature of being able to distribute 
vmbus 
interrupt load across all VCPUs in the guest is a win8 and beyond feature. On 
prior
hosts, all interrupts will be delivered on the boot CPU. VMBUS, as part of 
connecting with
the hosts determines host supported protocol version and decides how it wants to
program the hypervisor with regards to interrupt delivery. Even though we might 
setup
an IDT entry for delivering the hypervisor interrupt, if the host is a pre-win8 
host, the vmbus
driver will program the hypervisor to deliver the interrupt on the boot CPU via 
a legacy interrupt
vector.

Regards,

K. Y

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 2/3] X86: Add a check to catch Xen emulation of Hyper-V

[KY Srinivasan]

> -Original Message-
> From: devel [mailto:devel-boun...@linuxdriverproject.org] On Behalf Of KY
> Srinivasan
> Sent: Friday, February 01, 2013 10:11 AM
> To: Stefano Stabellini; H. Peter Anvin
> Cc: o...@aepfle.de; gre...@linuxfoundation.org; jasow...@redhat.com;
> x...@kernel.org; linux-kernel@vger.kernel.org; b...@alien8.de; Jan Beulich;
> a...@canonical.com; de...@linuxdriverproject.org; t...@linutronix.de
> Subject: RE: [PATCH 2/3] X86: Add a check to catch Xen emulation of Hyper-V
> 
> 
> 
> > -Original Message-
> > From: Stefano Stabellini [mailto:stefano.stabell...@eu.citrix.com]
> > Sent: Friday, February 01, 2013 8:20 AM
> > To: H. Peter Anvin
> > Cc: Jan Beulich; KY Srinivasan; o...@aepfle.de; b...@alien8.de;
> > a...@canonical.com; x...@kernel.org; t...@linutronix.de;
> > de...@linuxdriverproject.org; gre...@linuxfoundation.org;
> > jasow...@redhat.com; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH 2/3] X86: Add a check to catch Xen emulation of Hyper-V
> >
> > On Thu, 31 Jan 2013, H. Peter Anvin wrote:
> > > On 01/30/2013 12:53 AM, Jan Beulich wrote:
> > > >
> > > > I'm not convinced that's the right approach - any hypervisor
> > > > could do similar emulation, and hence you either want to make
> > > > sure you run on Hyper-V (by excluding all others), or you
> > > > tolerate using the emulation (which may require syncing up with
> > > > the other guest implementations so that shared resources don't
> > > > get used by two parties).
> > > >
> > > > I also wonder whether using the Hyper-V emulation (where
> > > > useful, there might not be anything right now, but this may
> > > > change going forward) when no Xen support is configured
> > > > wouldn't be better than not using anything...
> > > >
> > >
> > > I'm confused about "the right approach" here is.  As far as I
> > > understand, this only can affect a Xen guest where HyperV guest support
> > > is enabled but not Xen support, and only because Xen emulates HyperV but
> > > does so incorrectly.
> > >
> > > This is a Xen bug, and as such it makes sense to reject Xen
> > > specifically.  If another hypervisor emulates HyperV and does so
> > > correctly there seems to be no reason to reject it.
> >
> > I don't think so.
> >
> > AFAIK originally there were features exported as flags and Xen doesn't
> > turn on the flags that correspond to features that are not implemented.
> > The problem here is that Hyper-V is about to introduce a feature without
> > a flag that is not implemented by Xen (see "X86: Deliver Hyper-V
> > interrupts on a separate IDT vector").
> > K.Y. please confirm if I got this right.
> 
> I am not sure I can agree with you here. There are two discriminating factors
> here: (a) Hypervisor check and (b) Feature check. Not every feature of the
> hypervisor can be surfaced as feature bit and furthermore, just because a
> feature
> bit is turned on, it does not necessarily mean that the feature is to be 
> used. For
> instance,
> let us say that Windows guests begin to use the "partition counter" and Xen
> chooses
> to implement that to better support Windows. This does not mean that while
> hosting
> Linux on Xen, you want to plug in a clock source based on the emulated
> "partition counter". This is what would happen in the code we have today.
> 
> Other Hypervisors emulating Hyper-V do not have this problem and Xen would
> not either
> if the emulation bit is selectively turned on (only while running Windows) or 
> if
> Xen were allowed
> to check first ahead of Hyper-V (unconditionally) in the hypervisor detection
> code. As Peter pointed out, we
> have this problem because of the unique situation with Xen.
> 
> In any event, I am not going to further argue this issue; this last round of 
> patches I
> sent out,
> fixes the issue for Xen. Jan wants me to make this check more general. While I
> don't think
> we need to do that, I will see if I can do it. I am checking to see if MSFT
> guarantees that Hyper-V
> would initialize the unused CPUID space to 0. If this is the case, I will 
> implement
> the check
> Jan has suggested; if not, we have to live with the Xen specific check that I
> currently have.

I checked with the Hyper-V guys and I am told that there is no guarantee that 
Hyper-V
would not use some other range in the CPUID space in the future. So, I will 
keep the Xen
specific check that I had in this versi

RE: [PATCH RESEND 1/1] X86: Handle Hyper-V vmbus interrupts as special hypervisor interrupts

[KY Srinivasan]

> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: Thursday, January 24, 2013 3:48 AM
> To: KY Srinivasan
> Cc: o...@aepfle.de; b...@alien8.de; a...@canonical.com; x...@kernel.org;
> t...@linutronix.de; de...@linuxdriverproject.org; gre...@linuxfoundation.org;
> jasow...@redhat.com; linux-kernel@vger.kernel.org; h...@zytor.com
> Subject: Re: [PATCH RESEND 1/1] X86: Handle Hyper-V vmbus interrupts as
> special hypervisor interrupts
> 
> >>> On 24.01.13 at 02:17, "K. Y. Srinivasan"  wrote:
> > @@ -69,6 +74,11 @@ static void __init ms_hyperv_init_platform(void)
> >ms_hyperv.features, ms_hyperv.hints);
> >
> > clocksource_register_hz(&hyperv_cs, NSEC_PER_SEC/100);
> > +
> > +   /*
> > +* Setup the IDT for hypervisor callback.
> > +*/
> > +   alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR,
> hyperv_callback_vector);
> 
> Isn't doing this unconditionally here as problematic as the call to
> clocksource_register_hz() turned out to be when Xen's Hyper-V
> shim reacts to the CPUID inquiry above?

I was not sure what to make this conditional on at run-time. To the extent
that Xen emulation of Hyper-V is complete, this will be a problem. Does Xen
return all the "feature bits" of Hyper-V. I could discriminate on a feature that
Xen does not plan to emulate.

> 
> > @@ -77,3 +87,32 @@ const __refconst struct hypervisor_x86
> x86_hyper_ms_hyperv = {
> > .init_platform  = ms_hyperv_init_platform,
> >  };
> >  EXPORT_SYMBOL(x86_hyper_ms_hyperv);
> > +
> > +static int vmbus_irq = -1;
> > +static irq_handler_t vmbus_isr;
> > +
> > +void hv_register_vmbus_handler(int irq, irq_handler_t handler)
> > +{
> > +   vmbus_irq = irq;
> > +   vmbus_isr = handler;
> > +}
> > +EXPORT_SYMBOL_GPL(hv_register_vmbus_handler);
> > +
> > +void hyperv_vector_handler(struct pt_regs *regs)
> > +{
> > +   struct pt_regs *old_regs = set_irq_regs(regs);
> > +   struct irq_desc *desc;
> > +
> > +   irq_enter();
> > +#ifdef CONFIG_X86
> > +   exit_idle();
> > +#endif
> 
> This being in a file underneath arch/x86 - why the conditional?

Good point, I will fix that.
> 
> Jan
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 6/6] Drivers: hv: Execute shutdown in a thread context

[KY Srinivasan]

> -Original Message-
> From: Jiri Kosina [mailto:jkos...@suse.cz]
> Sent: Thursday, January 24, 2013 5:10 AM
> To: KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com; da...@davemloft.net;
> james.bottom...@hansenpartnership.com
> Subject: Re: [PATCH 6/6] Drivers: hv: Execute shutdown in a thread context
> 
> On Wed, 23 Jan 2013, K. Y. Srinivasan wrote:
> 
> > Execute the shutdown code in a thread context. With recent changes made to
> the
> > shutdown code, shutdown code cannot be invoked from an interrupt context.
> >
> > Signed-off-by: K. Y. Srinivasan 
> > Reviewed-by: Haiyang Zhang 
> > ---
> >  drivers/hv/hv_util.c |   12 +++-
> >  1 files changed, 11 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
> > index 8b7868a..aceb67f 100644
> > --- a/drivers/hv/hv_util.c
> > +++ b/drivers/hv/hv_util.c
> > @@ -49,6 +49,16 @@ static struct hv_util_service util_kvp = {
> > .util_deinit = hv_kvp_deinit,
> >  };
> >
> > +static void perform_shutdown(struct work_struct *dummy)
> > +{
> > +   orderly_poweroff(true);
> > +}
> 
> Is there any particular reason for this kind of crazy indentation?
I don't know how this extra tab crept through! Greg, if you want I can resend
this patch minus the extra tab. Let me know.

Regards,

K. Y
> 
> --
> Jiri Kosina
> SUSE Labs
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 6/6] Drivers: hv: Execute shutdown in a thread context

[KY Srinivasan]

> -Original Message-
> From: gre...@linuxfoundation.org [mailto:gre...@linuxfoundation.org]
> Sent: Thursday, January 24, 2013 12:18 PM
> To: KY Srinivasan
> Cc: Jiri Kosina; o...@aepfle.de; jasow...@redhat.com; linux-
> ker...@vger.kernel.org; james.bottom...@hansenpartnership.com;
> a...@canonical.com; de...@linuxdriverproject.org; da...@davemloft.net
> Subject: Re: [PATCH 6/6] Drivers: hv: Execute shutdown in a thread context
> 
> On Thu, Jan 24, 2013 at 05:06:27PM +, KY Srinivasan wrote:
> >
> >
> > > -Original Message-
> > > From: Jiri Kosina [mailto:jkos...@suse.cz]
> > > Sent: Thursday, January 24, 2013 5:10 AM
> > > To: KY Srinivasan
> > > Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> > > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> > > jasow...@redhat.com; da...@davemloft.net;
> > > james.bottom...@hansenpartnership.com
> > > Subject: Re: [PATCH 6/6] Drivers: hv: Execute shutdown in a thread context
> > >
> > > On Wed, 23 Jan 2013, K. Y. Srinivasan wrote:
> > >
> > > > Execute the shutdown code in a thread context. With recent changes made
> to
> > > the
> > > > shutdown code, shutdown code cannot be invoked from an interrupt
> context.
> > > >
> > > > Signed-off-by: K. Y. Srinivasan 
> > > > Reviewed-by: Haiyang Zhang 
> > > > ---
> > > >  drivers/hv/hv_util.c |   12 +++-
> > > >  1 files changed, 11 insertions(+), 1 deletions(-)
> > > >
> > > > diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
> > > > index 8b7868a..aceb67f 100644
> > > > --- a/drivers/hv/hv_util.c
> > > > +++ b/drivers/hv/hv_util.c
> > > > @@ -49,6 +49,16 @@ static struct hv_util_service util_kvp = {
> > > > .util_deinit = hv_kvp_deinit,
> > > >  };
> > > >
> > > > +static void perform_shutdown(struct work_struct *dummy)
> > > > +{
> > > > +   orderly_poweroff(true);
> > > > +}
> > >
> > > Is there any particular reason for this kind of crazy indentation?
> > I don't know how this extra tab crept through! Greg, if you want I can 
> > resend
> > this patch minus the extra tab. Let me know.
> 
> I'll edit it by hand, but someone owes me a beer for it...  :)

You got it!

Regards,

K. Y
> 
> greg k-h
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH RESEND 1/1] X86: Handle Hyper-V vmbus interrupts as special hypervisor interrupts

[KY Srinivasan]

> -Original Message-
> From: Borislav Petkov [mailto:b...@alien8.de]
> Sent: Thursday, January 24, 2013 4:28 AM
> To: KY Srinivasan
> Cc: x...@kernel.org; gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com; t...@linutronix.de; h...@zytor.com; jbeul...@suse.com
> Subject: Re: [PATCH RESEND 1/1] X86: Handle Hyper-V vmbus interrupts as
> special hypervisor interrupts
> 
> On Wed, Jan 23, 2013 at 05:56:09PM -0800, K. Y. Srinivasan wrote:
> > diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
> > index 1975122..803ca69 100644
> > --- a/arch/x86/kernel/entry_64.S
> > +++ b/arch/x86/kernel/entry_64.S
> > @@ -1446,11 +1446,16 @@ ENTRY(xen_failsafe_callback)
> > CFI_ENDPROC
> >  END(xen_failsafe_callback)
> >
> > -apicinterrupt XEN_HVM_EVTCHN_CALLBACK \
> > +apicinterrupt HYPERVISOR_CALLBACK_VECTOR \
> > xen_hvm_callback_vector xen_evtchn_do_upcall
> >
> >  #endif /* CONFIG_XEN */
> >
> > +#if IS_ENABLED(CONFIG_HYPERV)
> > +apicinterrupt HYPERVISOR_CALLBACK_VECTOR \
> > +   hyperv_callback_vector hyperv_vector_handler
> > +#endif /* CONFIG_HYPERV */
> 
> arch/x86/built-in.o: In function `_set_gate':
> /w/kernel/linux-2.6/arch/x86/include/asm/desc.h:328: undefined reference to
> `hyperv_callback_vector'
> make: *** [vmlinux] Error 1
> 
> because, of course:
> 
> # CONFIG_HYPERV is not set

My mistake. I should have properly guarded code that needs to be conditional. 
This also would address
the issue that Jan raised. I will resend this patch soon.

Regards,

K. Y 

> 
> But, I have a more serious pet-peeve with the whole hypervisors
> detection stuff: we're building arch/x86/kernel/cpu/hypervisor.c
> unconditionally and yet, we have CONFIG_PARAVIRT_GUEST to ask the user
> whether she wants to enable some options for running linux as a guest.
> 
> And actually, it would be better to put all that virt-related stuff
> under a config option called HYPERVISOR or whatever, under "Processor
> type and features" which opens a menu with all virt stuff for people and
> distros to select.
> 
> This way, init_hypervisor_platform and the rest of hypervisors stuff
> won't run needlessly on baremetal and setups who don't want that.
> 
> Any non-starter reasons for not doing that?


> 
> --
> Regards/Gruss,
> Boris.
> 
> Sent from a fat crate under my desk. Formatting is fine.
> --
> 

RE: [PATCH RESEND 1/1] X86: Handle Hyper-V vmbus interrupts as special hypervisor interrupts

[KY Srinivasan]

> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: Thursday, January 24, 2013 11:16 AM
> To: KY Srinivasan
> Cc: o...@aepfle.de; b...@alien8.de; a...@canonical.com; x...@kernel.org;
> t...@linutronix.de; de...@linuxdriverproject.org; gre...@linuxfoundation.org;
> jasow...@redhat.com; linux-kernel@vger.kernel.org; h...@zytor.com
> Subject: RE: [PATCH RESEND 1/1] X86: Handle Hyper-V vmbus interrupts as
> special hypervisor interrupts
> 
> >>> On 24.01.13 at 17:07, KY Srinivasan  wrote:
> 
> >
> >> -Original Message-
> >> From: Jan Beulich [mailto:jbeul...@suse.com]
> >> Sent: Thursday, January 24, 2013 3:48 AM
> >> To: KY Srinivasan
> >> Cc: o...@aepfle.de; b...@alien8.de; a...@canonical.com; x...@kernel.org;
> >> t...@linutronix.de; de...@linuxdriverproject.org;
> > gre...@linuxfoundation.org;
> >> jasow...@redhat.com; linux-kernel@vger.kernel.org; h...@zytor.com
> >> Subject: Re: [PATCH RESEND 1/1] X86: Handle Hyper-V vmbus interrupts as
> >> special hypervisor interrupts
> >>
> >> >>> On 24.01.13 at 02:17, "K. Y. Srinivasan"  wrote:
> >> > @@ -69,6 +74,11 @@ static void __init ms_hyperv_init_platform(void)
> >> > ms_hyperv.features, ms_hyperv.hints);
> >> >
> >> >  clocksource_register_hz(&hyperv_cs, NSEC_PER_SEC/100);
> >> > +
> >> > +/*
> >> > + * Setup the IDT for hypervisor callback.
> >> > + */
> >> > +alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR,
> >> hyperv_callback_vector);
> >>
> >> Isn't doing this unconditionally here as problematic as the call to
> >> clocksource_register_hz() turned out to be when Xen's Hyper-V
> >> shim reacts to the CPUID inquiry above?
> >
> > I was not sure what to make this conditional on at run-time. To the extent
> > that Xen emulation of Hyper-V is complete, this will be a problem. Does Xen
> > return all the "feature bits" of Hyper-V. I could discriminate on a feature
> > that
> > Xen does not plan to emulate.
> 
> I've no idea what plans there might be, but that's the code
> there is currently:
> 
> int cpuid_viridian_leaves(unsigned int leaf, unsigned int *eax,
>   unsigned int *ebx, unsigned int *ecx,
>   unsigned int *edx)
> {
> struct domain *d = current->domain;
> 
> if ( !is_viridian_domain(d) )
> return 0;
> 
> leaf -= 0x4000;
> if ( leaf > 6 )
> return 0;
> 
> *eax = *ebx = *ecx = *edx = 0;
> switch ( leaf )
> {
> case 0:
> *eax = 0x4006; /* Maximum leaf */
> *ebx = 0x7263694d; /* Magic numbers  */
> *ecx = 0x666F736F;
> *edx = 0x76482074;
> break;
> case 1:
> *eax = 0x31237648; /* Version number */
> break;
> case 2:
> /* Hypervisor information, but only if the guest has set its
>own version number. */
> if ( d->arch.hvm_domain.viridian.guest_os_id.raw == 0 )
> break;
> *eax = 1; /* Build number */
> *ebx = (xen_major_version() << 16) | xen_minor_version();
> *ecx = 0; /* SP */
> *edx = 0; /* Service branch and number */
> break;
> case 3:
> /* Which hypervisor MSRs are available to the guest */
> *eax = (CPUID3A_MSR_APIC_ACCESS |
> CPUID3A_MSR_HYPERCALL   |
> CPUID3A_MSR_VP_INDEX);
> break;
> case 4:
> /* Recommended hypercall usage. */
> if ( (d->arch.hvm_domain.viridian.guest_os_id.raw == 0) ||
>  (d->arch.hvm_domain.viridian.guest_os_id.fields.os < 4) )
> break;
> *eax = (CPUID4A_MSR_BASED_APIC |
> CPUID4A_RELAX_TIMER_INT);
> *ebx = 2047; /* long spin count */
> break;
> }
> 
> return 1;
> }
> 
> Question is - considering you stated that this is supported
> starting in Win8, doesn't Hyper-V itself announce that
> capability in some explicit way?

Thanks Jan. Unfortunately I don't think tis interrupt delivery model is
specified via a "feature" bit (I will check with the Windows guys). Perhaps, we
can have a Hyper-V specific compilation switch to address the Xen emulation 
issue.

Regards,

K. Y  
> 
> Jan
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH RESEND 1/1] X86: Handle Hyper-V vmbus interrupts as special hypervisor interrupts

[KY Srinivasan]

> -Original Message-
> From: Olaf Hering [mailto:o...@aepfle.de]
> Sent: Thursday, January 24, 2013 2:00 PM
> To: KY Srinivasan
> Cc: Jan Beulich; b...@alien8.de; a...@canonical.com; x...@kernel.org;
> t...@linutronix.de; de...@linuxdriverproject.org; gre...@linuxfoundation.org;
> jasow...@redhat.com; linux-kernel@vger.kernel.org; h...@zytor.com
> Subject: Re: [PATCH RESEND 1/1] X86: Handle Hyper-V vmbus interrupts as
> special hypervisor interrupts
> 
> On Thu, Jan 24, KY Srinivasan wrote:
> 
> 
> > > Question is - considering you stated that this is supported
> > > starting in Win8, doesn't Hyper-V itself announce that
> > > capability in some explicit way?
> >
> > Thanks Jan. Unfortunately I don't think tis interrupt delivery model
> > is specified via a "feature" bit (I will check with the Windows guys).
> > Perhaps, we can have a Hyper-V specific compilation switch to address
> > the Xen emulation issue.
> 
> Would that really help if both pvops XEN_PVHVM and HYPERV are enabled in
> the config? I assume at this point the access to the DMI data is not yet
> possible.

I was under the impression that only Dom0 Xen would emulate Hyper-V.


K. Y
> 
> Olaf
> 

RE: [PATCH] x86: Hyper-V: register clocksource only if its advertised

[KY Srinivasan]

> -Original Message-
> From: Olaf Hering [mailto:o...@aepfle.de]
> Sent: Friday, January 25, 2013 8:37 AM
> To: linux-kernel@vger.kernel.org
> Cc: Olaf Hering; sta...@kernel.org; KY Srinivasan; Greg KH
> Subject: [PATCH] x86: Hyper-V: register clocksource only if its advertised
> 
> Enable hyperv_clocksource only if its advertised as a feature.
> XenServer 6 returns the signature which is checked in
> ms_hyperv_platform(), but it does not offer all features. Currently the
> clocksource is enabled unconditionally in ms_hyperv_init_platform(), and
> the result is a hanging guest.
> 
> Hyper-V spec Bit 1 indicates the availability of Partition Reference
> Counter.  Register the clocksource only if this bit is set.
> 
> The guest in question prints this in dmesg:
>  [0.00] Hypervisor detected: Microsoft HyperV
>  [0.00] HyperV: features 0x70, hints 0x0
> 
> This bug can be reproduced easily be setting 'viridian=1' in a HVM domU
> .cfg file. A workaround without this patch is to boot the HVM guest with
> 'clocksource=jiffies'.

I am curious why you are setting viridian=1 for a Linux HVM guest. Initially 
when
I did the Hyper-V emulation work on Xen, this configuration flag would be set 
only
for Windows guests since the idea was to run the "enlightened" Windows on Xen.
While the current Xen emulation code does not emulate "Partition Reference 
Counter"
there is no guarantee that Xen would not do this in the future. Perhaps dealing 
with this issue
by identifying the guest (via the viridian domain or not) may be a safer 
approach?

Regards,

K. Y 
> 
> Signed-off-by: Olaf Hering 
> Cc: sta...@kernel.org
> Cc: KY Srinivasan 
> Cc: Greg KH 
> ---
>  arch/x86/kernel/cpu/mshyperv.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 0a630dd..646d192 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -68,7 +68,8 @@ static void __init ms_hyperv_init_platform(void)
>   printk(KERN_INFO "HyperV: features 0x%x, hints 0x%x\n",
>  ms_hyperv.features, ms_hyperv.hints);
> 
> - clocksource_register_hz(&hyperv_cs, NSEC_PER_SEC/100);
> + if (ms_hyperv.features & HV_X64_MSR_TIME_REF_COUNT_AVAILABLE)
> + clocksource_register_hz(&hyperv_cs, NSEC_PER_SEC/100);
>  }
> 
>  const __refconst struct hypervisor_x86 x86_hyper_ms_hyperv = {
> --
> 1.8.1.1
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] x86: Hyper-V: register clocksource only if its advertised

[KY Srinivasan]

> -Original Message-
> From: Olaf Hering [mailto:o...@aepfle.de]
> Sent: Friday, January 25, 2013 11:55 AM
> To: KY Srinivasan
> Cc: linux-kernel@vger.kernel.org; Greg KH
> Subject: Re: [PATCH] x86: Hyper-V: register clocksource only if its advertised
> 
> On Fri, Jan 25, KY Srinivasan wrote:
> 
> >
> >
> > > -Original Message-
> > > From: Olaf Hering [mailto:o...@aepfle.de]
> > > Sent: Friday, January 25, 2013 8:37 AM
> > > To: linux-kernel@vger.kernel.org
> > > Cc: Olaf Hering; sta...@kernel.org; KY Srinivasan; Greg KH
> > > Subject: [PATCH] x86: Hyper-V: register clocksource only if its advertised
> > >
> > > Enable hyperv_clocksource only if its advertised as a feature.
> > > XenServer 6 returns the signature which is checked in
> > > ms_hyperv_platform(), but it does not offer all features. Currently the
> > > clocksource is enabled unconditionally in ms_hyperv_init_platform(), and
> > > the result is a hanging guest.
> > >
> > > Hyper-V spec Bit 1 indicates the availability of Partition Reference
> > > Counter.  Register the clocksource only if this bit is set.
> > >
> > > The guest in question prints this in dmesg:
> > >  [0.00] Hypervisor detected: Microsoft HyperV
> > >  [0.00] HyperV: features 0x70, hints 0x0
> > >
> > > This bug can be reproduced easily be setting 'viridian=1' in a HVM domU
> > > .cfg file. A workaround without this patch is to boot the HVM guest with
> > > 'clocksource=jiffies'.
> >
> > I am curious why you are setting viridian=1 for a Linux HVM guest. 
> > Initially when
> > I did the Hyper-V emulation work on Xen, this configuration flag would be 
> > set
> only
> > for Windows guests since the idea was to run the "enlightened" Windows on
> Xen.
> > While the current Xen emulation code does not emulate "Partition Reference
> Counter"
> > there is no guarantee that Xen would not do this in the future. Perhaps 
> > dealing
> with this issue
> > by identifying the guest (via the viridian domain or not) may be a safer
> approach?
> 
> I'm not sure if XenServer does that per default, but I used the
> viridian= flag to reproduce the report.
> If Xen starts to emulate the "Partition Reference Counter" it should
> also advertise this emulation with the feature flag. Are you saying that
> checking for features is not the right approach?

My fear is that there is no guarantee that Xen would not emulate this feature
in the spirit of making Hyper-V emulation "more" complete. Since all this 
problem is
because Xen thinks it is running a viridian domain (when in fact it is running 
Linux), I felt we
should explore why the viridian tag got set for a Linux VM. If we can fix that 
we would have
a solution that does not depend upon assuming that Xen would not emulate a 
particular Hyper-V
feature.

K. Y 

RE: [PATCH] x86: Hyper-V: register clocksource only if its advertised

[KY Srinivasan]

> -Original Message-
> From: Olaf Hering [mailto:o...@aepfle.de]
> Sent: Friday, January 25, 2013 12:19 PM
> To: KY Srinivasan
> Cc: linux-kernel@vger.kernel.org; Greg KH; Jan Beulich (jbeul...@suse.com)
> Subject: Re: [PATCH] x86: Hyper-V: register clocksource only if its advertised
> 
> On Fri, Jan 25, KY Srinivasan wrote:
> 
> > My fear is that there is no guarantee that Xen would not emulate this
> > feature in the spirit of making Hyper-V emulation "more" complete.
> > Since all this problem is because Xen thinks it is running a viridian
> > domain (when in fact it is running Linux), I felt we should explore
> > why the viridian tag got set for a Linux VM. If we can fix that we
> > would have a solution that does not depend upon assuming that Xen
> > would not emulate a particular Hyper-V feature.
> 
> In my opinion this logic is backwards. A host provides a set of
> functionality/features to a given guest, no matter what runs inside the
> guest. And the guest can query the feature list and configure itself
> accordingly. In this specific case the guest finds feature A (the cpuid
> result) and expects that feature B (a clock source) is present as well,
> even if feature B has its very own availability flag.

I would agree with you in the abstract but not in this specific case. First,
the current Hyper-V code in Linux verifies that the underlying hypervisor is
Hyper-V and proceeds to setup state that is needed to run Linux on Hyper-V.
If Xen were not emulating Hyper-V, we would not have any issue here. However,
Xen does emulate Hyper-V and it is emulating Hyper-V not to run Linux, but to 
run
enlightened Windows (making Windows think it is running on Hyper-V). We have a 
mechanism in place for controlling this Hyper-V emulation in Xen - the viridian 
flag.
In my view, ensuring that this flag is not set for a non -Windows guest is the 
only safe
approach to dealing with this issue. Your current approach of using the 
"Partition Reference
Counter" feature bit is very fragile. Let us assume that future Windows guests 
depend on this
feature; Xen would certainly emulate this to have a "good" emulation of Hyper-V 
and we are
back to square one.

Regards,

K. Y
> 
> Maybe I miss your point.
> 
> Olaf
> 

RE: [PATCH] x86: Hyper-V: register clocksource only if its advertised

[KY Srinivasan]

> -Original Message-
> From: Olaf Hering [mailto:o...@aepfle.de]
> Sent: Friday, January 25, 2013 3:00 PM
> To: KY Srinivasan
> Cc: linux-kernel@vger.kernel.org; Greg KH; Jan Beulich (jbeul...@suse.com)
> Subject: Re: [PATCH] x86: Hyper-V: register clocksource only if its advertised
> 
> On Fri, Jan 25, KY Srinivasan wrote:
> 
> > Your current approach of using the "Partition Reference Counter"
> > feature bit is very fragile. Let us assume that future Windows guests
> > depend on this feature; Xen would certainly emulate this to have a
> > "good" emulation of Hyper-V and we are back to square one.
> 
> If Xen starts to emulate the "Partition Reference Counter" feature,
> wouldnt it also advertise this feature?

Yes; it has to - if it expects the Windows guests to use it. This would be
no different than the other features that Xen emulates and advertises.

K. Y
 

N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�&j:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

RE: [tip:x86/asm] x86/xor: Make virtualization friendly

[KY Srinivasan]

> -Original Message-
> From: H. Peter Anvin [mailto:h...@zytor.com]
> Sent: Friday, January 25, 2013 8:05 PM
> To: H. Peter Anvin
> Cc: mi...@kernel.org; linux-kernel@vger.kernel.org; konrad.w...@oracle.com;
> torva...@linux-foundation.org; jbeul...@suse.com; t...@linutronix.de; linux-
> tip-comm...@vger.kernel.org; Marcelo Tosatti; KY Srinivasan; Haiyang Zhang
> Subject: Re: [tip:x86/asm] x86/xor: Make virtualization friendly
> 
> On 01/25/2013 02:15 PM, H. Peter Anvin wrote:
> > On 01/25/2013 02:11 PM, H. Peter Anvin wrote:
> >> On 01/25/2013 02:43 AM, tip-bot for Jan Beulich wrote:
> >>> Commit-ID:  05fbf4d6fc6a3c0c3e63b77979c9311596716d10
> >>> Gitweb:
> http://git.kernel.org/tip/05fbf4d6fc6a3c0c3e63b77979c9311596716d10
> >>> Author: Jan Beulich 
> >>> AuthorDate: Fri, 2 Nov 2012 14:21:23 +
> >>> Committer:  Ingo Molnar 
> >>> CommitDate: Fri, 25 Jan 2013 09:23:51 +0100
> >>>
> >>> x86/xor: Make virtualization friendly
> >>>
> >>> In virtualized environments, the CR0.TS management needed here
> >>> can be a lot slower than anticipated by the original authors of
> >>> this code, which particularly means that in such cases forcing
> >>> the use of SSE- (or MMX-) based implementations is not desirable
> >>> - actual measurements should always be done in that case.
> >>>
> >>> For consistency, pull into the shared (32- and 64-bit) header
> >>> not only the inclusion of the generic code, but also that of the
> >>> AVX variants.
> >>>
> >>
> >> This patch is wrong and should be dropped.  I verified it with the KVM
> >> people that they do NOT want this change.  It is a Xen-specific problem.
> >>
> >
> > FWIW: I have dropped this patch from tip:x86/asm.
> >
> 
> The bottom line, I guess, is that we need something like
> cpu_has_slow_kernel_fpu or something like that, and set it for
> specifically affected hypervisors?
> 
> Do we know if Hyper-V has performance issues with CR0.TS?

Checking with the Hyper-V developers, Hyper-V does not have performance issues
With CR0.TS

Regards,

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

RE: [PATCH] x86: Hyper-V: register clocksource only if its advertised

[KY Srinivasan]

> -Original Message-
> From: Olaf Hering [mailto:o...@aepfle.de]
> Sent: Friday, January 25, 2013 3:00 PM
> To: KY Srinivasan
> Cc: linux-kernel@vger.kernel.org; Greg KH; Jan Beulich (jbeul...@suse.com)
> Subject: Re: [PATCH] x86: Hyper-V: register clocksource only if its advertised
> 
> On Fri, Jan 25, KY Srinivasan wrote:
> 
> > Your current approach of using the "Partition Reference Counter"
> > feature bit is very fragile. Let us assume that future Windows guests
> > depend on this feature; Xen would certainly emulate this to have a
> > "good" emulation of Hyper-V and we are back to square one.
> 
> If Xen starts to emulate the "Partition Reference Counter" feature,
> wouldnt it also advertise this feature?

Olaf, did you get a chance to investigate why the viridian bit was set for the 
Linux HVM domain.
I am thinking of re-sending the patch for delivering the Hyper-V vmbus on a 
separate vector without
a run-time check. Let me know.

Regards,

K. Y 
> 
> Olaf
> 

RE: [PATCH 1/1] Drivers: hv: vmbus: Use the new infrastructure for delivering VMBUS interrupts

[KY Srinivasan]

Greg,

Can this patch go in now. I checked Linus' tree and it looks like all the 
needed patches are all checked in.

Regards,

K. Y

> -Original Message-
> From: K. Y. Srinivasan [mailto:k...@microsoft.com]
> Sent: Sunday, February 17, 2013 2:31 PM
> To: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com; h...@zytor.com
> Cc: KY Srinivasan
> Subject: [PATCH 1/1] Drivers: hv: vmbus: Use the new infrastructure for
> delivering VMBUS interrupts
> 
> Use the infrastructure for delivering VMBUS interrupts using a
> special vector. With this patch, we can now properly handle
> the VMBUS interrupts that can be delivered on any CPU. Also,
> turn on interrupt load balancing as well.
> 
> This patch requires the infrastructure that was implemented in the patch:
> X86: Handle Hyper-V vmbus interrupts as special hypervisor interrupts
> 
> Greg, Please apply this patch after 3.9-rc1.
> 
> Signed-off-by: K. Y. Srinivasan 
> ---
>  drivers/hv/channel_mgmt.c |2 +-
>  drivers/hv/hv.c   |5 ++---
>  drivers/hv/vmbus_drv.c|   11 +++
>  3 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index 53a8600..ff1be16 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -318,7 +318,7 @@ static u32 get_vp_index(uuid_le *type_guid)
>   return 0;
>   }
>   cur_cpu = (++next_vp % max_cpus);
> - return 0;
> + return cur_cpu;
>  }
> 
>  /*
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index 1c5481d..7311589 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -272,7 +272,7 @@ u16 hv_signal_event(void *con_id)
>   * retrieve the initialized message and event pages.  Otherwise, we create 
> and
>   * initialize the message and event pages.
>   */
> -void hv_synic_init(void *irqarg)
> +void hv_synic_init(void *arg)
>  {
>   u64 version;
>   union hv_synic_simp simp;
> @@ -281,7 +281,6 @@ void hv_synic_init(void *irqarg)
>   union hv_synic_scontrol sctrl;
>   u64 vp_index;
> 
> - u32 irq_vector = *((u32 *)(irqarg));
>   int cpu = smp_processor_id();
> 
>   if (!hv_context.hypercall_page)
> @@ -335,7 +334,7 @@ void hv_synic_init(void *irqarg)
>   rdmsrl(HV_X64_MSR_SINT0 + VMBUS_MESSAGE_SINT,
> shared_sint.as_uint64);
> 
>   shared_sint.as_uint64 = 0;
> - shared_sint.vector = irq_vector; /* HV_SHARED_SINT_IDT_VECTOR +
> 0x20; */
> + shared_sint.vector = HYPERVISOR_CALLBACK_VECTOR;
>   shared_sint.masked = false;
>   shared_sint.auto_eoi = true;
> 
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index cf19dfa..bf421e0 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -36,6 +36,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include "hyperv_vmbus.h"
> 
> 
> @@ -528,7 +529,6 @@ static void vmbus_flow_handler(unsigned int irq, struct
> irq_desc *desc)
>  static int vmbus_bus_init(int irq)
>  {
>   int ret;
> - unsigned int vector;
> 
>   /* Hypervisor initialization...setup hypercall page..etc */
>   ret = hv_init();
> @@ -558,13 +558,16 @@ static int vmbus_bus_init(int irq)
>*/
>   irq_set_handler(irq, vmbus_flow_handler);
> 
> - vector = IRQ0_VECTOR + irq;
> + /*
> +  * Register our interrupt handler.
> +  */
> + hv_register_vmbus_handler(irq, vmbus_isr);
> 
>   /*
> -  * Notify the hypervisor of our irq and
> +  * Initialize the per-cpu interrupt state and
>* connect to the host.
>*/
> - on_each_cpu(hv_synic_init, (void *)&vector, 1);
> + on_each_cpu(hv_synic_init, NULL, 1);
>   ret = vmbus_connect();
>   if (ret)
>   goto err_irq;
> --
> 1.7.4.1
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 5/6] Drivers: hv: balloon: Implement hot-add functionality

[KY Srinivasan]

> -Original Message-
> From: gre...@linuxfoundation.org [mailto:gre...@linuxfoundation.org]
> Sent: Tuesday, March 12, 2013 12:07 PM
> To: KY Srinivasan
> Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org; 
> o...@aepfle.de;
> a...@canonical.com; jasow...@redhat.com
> Subject: Re: [PATCH 5/6] Drivers: hv: balloon: Implement hot-add functionality
> 
> On Tue, Mar 12, 2013 at 02:54:23AM +, KY Srinivasan wrote:
> >
> >
> > > -Original Message-
> > > From: K. Y. Srinivasan [mailto:k...@microsoft.com]
> > > Sent: Friday, March 08, 2013 5:16 PM
> > > To: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> > > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> > > jasow...@redhat.com
> > > Cc: KY Srinivasan
> > > Subject: [PATCH 5/6] Drivers: hv: balloon: Implement hot-add functionality
> > >
> > > Implement the memory hot-add functionality. With this, Linux guests can 
> > > fully
> > > participate in the Dynamic Memory protocol implemented in the Windows
> hosts.
> >
> > Greg,
> >
> > I forgot to modify the Kconfig file to include the new dependency of the
> balloon driver on
> > MEMORY_HOTPLUG. Should I send a separate patch for  Kconfig, or resend this
> one patch
> > with the Kconfig changes folded in. Or, do you want me to resend the whole
> series.
> 
> Just resend the whole series, I'll drop this one.

Will do.

K. Y
> 
> greg k-h
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/1] Drivers: hv: Add a new driver to support host initiated backup

[KY Srinivasan]

> -Original Message-
> From: K. Y. Srinivasan [mailto:k...@microsoft.com]
> Sent: Tuesday, March 12, 2013 2:29 PM
> To: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com
> Cc: KY Srinivasan
> Subject: [PATCH 1/1] Drivers: hv: Add a new driver to support host initiated
> backup
> 
> This driver supports host initiated backup of the guest. On Windows guests,
> the host can generate application consistent backups using the Windows VSS
> framework. On Linux, we ensure that the backup will be file system consistent.
> This driver allows the host to initiate a  "Freeze" operation on all the 
> mounted
> file systems in the guest. Once the mounted file systems in the guest are 
> frozen,
> the host snapshots the guest's file systems. Once this is done, the guest's 
> file
> systems are "thawed".
> 
> This driver has a user-level component (daemon) that invokes the appropriate
> operation on all the mounted file systems in response to the requests from
> the host. The duration for which the guest is frozen is very short - a few 
> seconds.
> During this interval, the diff disk is comitted.
> 
> Signed-off-by: K. Y. Srinivasan 
> Reviewed-by: Haiyang Zhang 

Greg,

Please drop this patch. I sent the wrong patch.

K. Y
> ---
>  drivers/hv/Makefile|2 +-
>  drivers/hv/hv_util.c   |   10 ++
>  include/linux/hyperv.h |   69
> 
>  include/uapi/linux/connector.h |5 ++-
>  4 files changed, 84 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile
> index e6abfa0..0a74b56 100644
> --- a/drivers/hv/Makefile
> +++ b/drivers/hv/Makefile
> @@ -5,4 +5,4 @@ obj-$(CONFIG_HYPERV_BALLOON)  += hv_balloon.o
>  hv_vmbus-y := vmbus_drv.o \
>hv.o connection.o channel.o \
>channel_mgmt.o ring_buffer.o
> -hv_utils-y := hv_util.o hv_kvp.o
> +hv_utils-y := hv_util.o hv_kvp.o hv_snapshot.o
> diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
> index 1d4cbd8..2f561c5 100644
> --- a/drivers/hv/hv_util.c
> +++ b/drivers/hv/hv_util.c
> @@ -49,6 +49,12 @@ static struct hv_util_service util_kvp = {
>   .util_deinit = hv_kvp_deinit,
>  };
> 
> +static struct hv_util_service util_vss = {
> + .util_cb = hv_vss_onchannelcallback,
> + .util_init = hv_vss_init,
> + .util_deinit = hv_vss_deinit,
> +};
> +
>  static void perform_shutdown(struct work_struct *dummy)
>  {
>   orderly_poweroff(true);
> @@ -339,6 +345,10 @@ static const struct hv_vmbus_device_id id_table[] = {
>   { HV_KVP_GUID,
> .driver_data = (unsigned long)&util_kvp
>   },
> + /* VSS GUID */
> + { HV_VSS_GUID,
> +   .driver_data = (unsigned long)&util_vss
> + },
>   { },
>  };
> 
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index df77ba9..95d0850 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -27,6 +27,63 @@
> 
>  #include 
> 
> +
> +/*
> + * Implementation of host controlled snapshot of the guest.
> + */
> +
> +#define VSS_OP_REGISTER 128
> +
> +enum hv_vss_op {
> + VSS_OP_CREATE = 0,
> + VSS_OP_DELETE,
> + VSS_OP_HOT_BACKUP,
> + VSS_OP_GET_DM_INFO,
> + VSS_OP_BU_COMPLETE,
> + /*
> +  * Following operations are only supported with IC version >= 5.0
> +  */
> + VSS_OP_FREEZE, /* Freeze the file systems in the VM */
> + VSS_OP_THAW, /* Unfreeze the file systems */
> + VSS_OP_AUTO_RECOVER,
> + VSS_OP_COUNT /* Number of operations, must be last */
> +};
> +
> +
> +/*
> + * Header for all VSS messages.
> + */
> +struct hv_vss_hdr {
> + __u8 operation;
> + __u8 reserved[7];
> +} __attribute__((packed));
> +
> +
> +/*
> + * Flag values for the hv_vss_check_feature. Linux supports only
> + * one value.
> + */
> +#define VSS_HBU_NO_AUTO_RECOVERY 0x0005
> +
> +struct hv_vss_check_feature {
> + __u32 flags;
> +} __attribute__((packed));
> +
> +struct hv_vss_check_dm_info {
> + __u32 flags;
> +} __attribute__((packed));
> +
> +struct hv_vss_msg {
> + union {
> + struct hv_vss_hdr vss_hdr;
> + int error;
> + };
> + union {
> + struct hv_vss_check_feature vss_cf;
> + struct hv_vss_check_dm_info dm_info;
> + };
> +} __attribute__((packed));
> +
>  /*
>   * An implementation of HyperV key value pair (KVP) functionality for Linux.
>   *
> @@ 

RE: [PATCH 1/1] Drivers: hv: Add a new driver to support host initiated backup

[KY Srinivasan]

> -Original Message-
> From: Olaf Hering [mailto:o...@aepfle.de]
> Sent: Tuesday, March 12, 2013 2:49 PM
> To: KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; a...@canonical.com; jasow...@redhat.com;
> Evgeniy Polyakov
> Subject: Re: [PATCH 1/1] Drivers: hv: Add a new driver to support host 
> initiated
> backup
> 
> On Tue, Mar 12, K. Y. Srinivasan wrote:
> 
> > +static int vss_operate(int operation)
> > +{
> > +   char *fs_op;
> > +   char cmd[512];
> > +   char buf[512];
> > +   FILE *file;
> > +   char *p;
> > +   char *x;
> > +   int error;
> > +
> > +   switch (operation) {
> > +   case VSS_OP_FREEZE:
> > +   fs_op = "-f ";
> > +   break;
> > +   case VSS_OP_THAW:
> > +   fs_op = "-u ";
> > +   break;
> > +   }
> > +
> > +   sprintf(cmd, "%s", "mount | grep ^/dev/ | awk '{print $3 }'");
> 
> I think this can be char cmd[] = "mount | awk '/^\/dev\/ { print $3'";

Yes; will make the change.
> 
> > +   file = popen(cmd, "r");
> > +   if (file == NULL)
> > +   return;
> > +
> > +   while ((p = fgets(buf, sizeof(buf), file)) != NULL) {
> > +   x = strchr(p, '\n');
> > +   *x = '\0';
> > +   if (!strncmp(p, "/", sizeof("/")))
> > +   continue;
> > +
> > +   sprintf(cmd, "%s %s %s", "fsfreeze ", fs_op, p);
> > +   syslog(LOG_INFO, "VSS cmd is %s\n", cmd);
> > +   error = system(cmd);
> 
> error is not handled here, and it looks like only one error can be
> reported anyway.
> In case of an error, will the host thaw the filesystems?

Both freeze and thaw operations will not fail if properly applied. The only 
time the
freeze operation fails is when there is already an active freeze in force. 
Likewise,
the only time thaw fails is when there has not been a preceding freeze 
operation. So these
errors will occur only when the VM user happens to be applying freeze/thaw 
operations
independent of the host. In this case, the host simply reports an error on the 
host on
the backup operation and nothing else is done.

Regards,

K. Y 
> 
> > +   }
> > +   pclose(file);
> > +
> > +   sprintf(cmd, "%s %s %s", "fsfreeze ", fs_op, "/");
> > +   syslog(LOG_INFO, "VSS cmd is %s\n", cmd);
> > +   error = system(cmd);
> > +
> > +   return error;
> > +}
> 
> 

RE: [PATCH RESEND 5/6] Drivers: hv: balloon: Implement hot-add functionality

[KY Srinivasan]

> -Original Message-
> From: Olaf Hering [mailto:o...@aepfle.de]
> Sent: Wednesday, March 13, 2013 12:50 PM
> To: KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; a...@canonical.com; jasow...@redhat.com
> Subject: Re: [PATCH RESEND 5/6] Drivers: hv: balloon: Implement hot-add
> functionality
> 
> On Tue, Mar 12, K. Y. Srinivasan wrote:
> 
> > Implement the memory hot-add functionality. With this, Linux guests can 
> > fully
> > participate in the Dynamic Memory protocol implemented in the Windows
> hosts.
> 
> > +++ b/drivers/hv/Kconfig
> > @@ -15,7 +15,7 @@ config HYPERV_UTILS
> >
> >  config HYPERV_BALLOON
> > tristate "Microsoft Hyper-V Balloon driver"
> > -   depends on HYPERV
> > +   depends on HYPERV && MEMORY_HOTPLUG
> > help
> >   Select this option to enable Hyper-V Balloon driver.
> 
> This driver now depends on CONFIG_MEMORY_HOTPLUG, which is disabled in
> openSuSE i386 kernel. As a result, the hv_balloon driver disappears for
> 32bit kernels.
> 
> Any chance that the Kconfig dependency can be moved inside the code so
> that a 32bit guest can still do balloon operations?

I can preserve the old behavior by getting rid of the config dependency and 
adding in
the MEMORY_HOTPLUG dependency around the affected code. Greg, shall I just 
re-send
this one patch.

Regards,

K. Y
> 
> 
> Olaf
> 

N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�&j:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

RE: [PATCH RESEND 0/6] Drivers: hv

[KY Srinivasan]

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Friday, March 15, 2013 2:21 PM
> To: KY Srinivasan
> Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org; 
> o...@aepfle.de;
> a...@canonical.com; jasow...@redhat.com
> Subject: Re: [PATCH RESEND 0/6] Drivers: hv
> 
> Your subject says "resend", but you don't tell me which version it is?
> I see about 3 different versions of this series, which one should I use?
> 
> Please be specific when sending patches, otherwise I will guess, and I
> usually guess wrong...
> 
> I'm going to drop this, and all other hv patches from you, please resend
> what I haven't applied yet.

I will send all the patches again.

K. Y
> 
> thanks,
> 
> greg k-h
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] hv: vmbus_drv: detect hyperv through x86_hyper

[KY Srinivasan]

> -Original Message-
> From: Jason Wang [mailto:jasow...@redhat.com]
> Sent: Friday, August 31, 2012 1:33 AM
> To: de...@linuxdriverproject.org; linux-kernel@vger.kernel.org
> Cc: Jason Wang; KY Srinivasan; Haiyang Zhang
> Subject: [PATCH] hv: vmbus_drv: detect hyperv through x86_hyper
> 
> There are two reasons we need to use x86_hyper instead of
> query_hypervisor_presence():
> 
> - Not only hyperv but also other hypervisors such as kvm would set
>   X86_FEATURE_HYTPERVISOR, so query_hypervisor_presence() will return true
> even
>   in kvm. This may cause extra delay of 5 seconds before failing the probing 
> in
>   kvm guest.
> - The hypervisor has been detected in init_hypervisor(), so no need to do the
>   work again.
> 
> Cc: "K. Y. Srinivasan" 
> Cc: Haiyang Zhang 
> Signed-off-by: Jason Wang 

Thanks Jason.

Acked-by: K. Y. Srinivasan 

> ---
>  drivers/hv/vmbus_drv.c |   25 ++---
>  1 files changed, 2 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index f40dd57..8e1a9ec 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -34,6 +34,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include "hyperv_vmbus.h"
> 
> 
> @@ -719,33 +720,11 @@ static struct acpi_driver vmbus_acpi_driver = {
>   },
>  };
> 
> -/*
> - * query_hypervisor_presence
> - * - Query the cpuid for presence of windows hypervisor
> - */
> -static int query_hypervisor_presence(void)
> -{
> - unsigned int eax;
> - unsigned int ebx;
> - unsigned int ecx;
> - unsigned int edx;
> - unsigned int op;
> -
> - eax = 0;
> - ebx = 0;
> - ecx = 0;
> - edx = 0;
> - op = HVCPUID_VERSION_FEATURES;
> - cpuid(op, &eax, &ebx, &ecx, &edx);
> -
> - return ecx & HV_PRESENT_BIT;
> -}
> -
>  static int __init hv_acpi_init(void)
>  {
>   int ret, t;
> 
> - if (!query_hypervisor_presence())
> + if (x86_hyper != &x86_hyper_ms_hyperv)
>   return -ENODEV;
> 
>   init_completion(&probe_event);
> --
> 1.7.1
> 
> 
> 



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/1] Drivers: hv: kvp: Copy the address family information

[KY Srinivasan]

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Tuesday, September 04, 2012 6:57 PM
> To: KY Srinivasan
> Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org; 
> o...@aepfle.de;
> a...@canonical.com; b...@decadent.org.uk
> Subject: Re: [PATCH 1/1] Drivers: hv: kvp: Copy the address family information
> 
> On Tue, Sep 04, 2012 at 03:20:19PM -0700, K. Y. Srinivasan wrote:
> > Copy the address family information.
> 
> Why?

This is part of the IP injection protocol in that the host expects this field 
to reflect what addresses
(address families) are currently bound to the interface. The KVP daemon is 
currently collecting this 
Information and sending it to the kernel component. I had overlooked copying 
this and sending it
back to the host. This patch addresses this issue.

Regards,

K. Y




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH V4 05/10] Tools: hv: Add an example script to retrieve dhcp state

[KY Srinivasan]

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Tuesday, September 04, 2012 6:59 PM
> To: KY Srinivasan
> Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org; 
> o...@aepfle.de;
> a...@canonical.com; b...@decadent.org.uk; tho...@redhat.com;
> d...@redhat.com
> Subject: Re: [PATCH V4 05/10] Tools: hv: Add an example script to retrieve 
> dhcp
> state
> 
> On Tue, Sep 04, 2012 at 02:46:37PM -0700, K. Y. Srinivasan wrote:
> > To keep the KVP daemon code free of distro specific details, we invoke an
> > external script to retrieve the DHCP state. This is an example script that
> > was used to test the KVP code. This script has to be implemented in a Distro
> > specific fashion. For instance on distros that ship with Network Manager
> enabled,
> > this script can be based on NM APIs.
> >
> > Signed-off-by: K. Y. Srinivasan 
> > Reviewed-by: Haiyang Zhang 
> > ---
> >  tools/hv/hv_get_dhcp_info.sh |   25 +
> >  1 files changed, 25 insertions(+), 0 deletions(-)
> >  create mode 100755 tools/hv/hv_get_dhcp_info.sh
> >
> > diff --git a/tools/hv/hv_get_dhcp_info.sh b/tools/hv/hv_get_dhcp_info.sh
> > new file mode 100755
> > index 000..3de4587
> > --- /dev/null
> > +++ b/tools/hv/hv_get_dhcp_info.sh
> > @@ -0,0 +1,25 @@
> > +#!/bin/bash
> > +
> > +# This example script retrieves the DHCP state of a given interface.
> > +# In the interest of keeping the KVP daemon code free of distro specific
> > +# information; the kvp daemon code invokes this external script to gather
> > +# DHCP setting for the specific interface.
> > +#
> > +# Input: Name of the interface
> > +#
> > +# Output: The script prints the string "Enabled" to stdout to indicate
> > +#  that DHCP is enabled on the interface.
> 
> What happens if DHCP is not enabled on the interface?  Shouldn't that
> also return something other than "success"?

The script is expected to write "Enabled" to stdout to indicate if DHCP is 
enabled; if this 
is not the case, implicitly we assume DHCP is not enabled (since this is a 
binary state).

Regards,

K. Y 



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH V4 05/10] Tools: hv: Add an example script to retrieve dhcp state

[KY Srinivasan]

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Tuesday, September 04, 2012 8:04 PM
> To: KY Srinivasan
> Cc: o...@aepfle.de; tho...@redhat.com; linux-kernel@vger.kernel.org;
> d...@redhat.com; a...@canonical.com; de...@linuxdriverproject.org;
> b...@decadent.org.uk
> Subject: Re: [PATCH V4 05/10] Tools: hv: Add an example script to retrieve 
> dhcp
> state
> 
> On Tue, Sep 04, 2012 at 11:39:12PM +, KY Srinivasan wrote:
> >
> >
> > > -Original Message-
> > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > Sent: Tuesday, September 04, 2012 6:59 PM
> > > To: KY Srinivasan
> > > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org;
> o...@aepfle.de;
> > > a...@canonical.com; b...@decadent.org.uk; tho...@redhat.com;
> > > d...@redhat.com
> > > Subject: Re: [PATCH V4 05/10] Tools: hv: Add an example script to retrieve
> dhcp
> > > state
> > >
> > > On Tue, Sep 04, 2012 at 02:46:37PM -0700, K. Y. Srinivasan wrote:
> > > > To keep the KVP daemon code free of distro specific details, we invoke 
> > > > an
> > > > external script to retrieve the DHCP state. This is an example script 
> > > > that
> > > > was used to test the KVP code. This script has to be implemented in a 
> > > > Distro
> > > > specific fashion. For instance on distros that ship with Network Manager
> > > enabled,
> > > > this script can be based on NM APIs.
> > > >
> > > > Signed-off-by: K. Y. Srinivasan 
> > > > Reviewed-by: Haiyang Zhang 
> > > > ---
> > > >  tools/hv/hv_get_dhcp_info.sh |   25 +
> > > >  1 files changed, 25 insertions(+), 0 deletions(-)
> > > >  create mode 100755 tools/hv/hv_get_dhcp_info.sh
> > > >
> > > > diff --git a/tools/hv/hv_get_dhcp_info.sh b/tools/hv/hv_get_dhcp_info.sh
> > > > new file mode 100755
> > > > index 000..3de4587
> > > > --- /dev/null
> > > > +++ b/tools/hv/hv_get_dhcp_info.sh
> > > > @@ -0,0 +1,25 @@
> > > > +#!/bin/bash
> > > > +
> > > > +# This example script retrieves the DHCP state of a given interface.
> > > > +# In the interest of keeping the KVP daemon code free of distro 
> > > > specific
> > > > +# information; the kvp daemon code invokes this external script to 
> > > > gather
> > > > +# DHCP setting for the specific interface.
> > > > +#
> > > > +# Input: Name of the interface
> > > > +#
> > > > +# Output: The script prints the string "Enabled" to stdout to indicate
> > > > +#  that DHCP is enabled on the interface.
> > >
> > > What happens if DHCP is not enabled on the interface?  Shouldn't that
> > > also return something other than "success"?
> >
> > The script is expected to write "Enabled" to stdout to indicate if DHCP is
> enabled; if this
> > is not the case, implicitly we assume DHCP is not enabled (since this is a 
> > binary
> state).
> 
> It's not really "binary" given that you are expecting "Enabled" or
> nothing, right?  "Disabled" would make a bit more sense perhaps?

In the KVP daemon code, I currently only check for "Enabled" to see if
DHCP is enabled.

> 
> Having a script return "nothing at all" to show the failure of a state
> doesn't seem the wisest thing, does it?

I can certainly resubmit this patch with the change you are suggesting. This 
patch adds a new file
and does not impact any of the other patches that follows this. Would you want 
me to resubmit all of the
remaining patches starting with this one.

Regards,

K. Y 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/1] Drivers: hv: kvp: Copy the address family information

[KY Srinivasan]

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Tuesday, September 04, 2012 8:03 PM
> To: KY Srinivasan
> Cc: a...@canonical.com; de...@linuxdriverproject.org; o...@aepfle.de;
> b...@decadent.org.uk; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/1] Drivers: hv: kvp: Copy the address family information
> 
> On Tue, Sep 04, 2012 at 11:36:30PM +, KY Srinivasan wrote:
> >
> >
> > > -Original Message-
> > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > Sent: Tuesday, September 04, 2012 6:57 PM
> > > To: KY Srinivasan
> > > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org;
> o...@aepfle.de;
> > > a...@canonical.com; b...@decadent.org.uk
> > > Subject: Re: [PATCH 1/1] Drivers: hv: kvp: Copy the address family
> information
> > >
> > > On Tue, Sep 04, 2012 at 03:20:19PM -0700, K. Y. Srinivasan wrote:
> > > > Copy the address family information.
> > >
> > > Why?
> >
> > This is part of the IP injection protocol in that the host expects this 
> > field to
> reflect what addresses
> > (address families) are currently bound to the interface. The KVP daemon is
> currently collecting this
> > Information and sending it to the kernel component. I had overlooked copying
> this and sending it
> > back to the host. This patch addresses this issue.
> 
> That's nice and descriptive, any reason you didn't put it in the patch
> changelog section the first time?
> 
> Please redo this...

Will do.

Regards,

K. Y
> 
> greg k-h
> 
> 



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH V4 05/10] Tools: hv: Add an example script to retrieve dhcp state

[KY Srinivasan]

> -Original Message-
> From: devel [mailto:devel-boun...@linuxdriverproject.org] On Behalf Of KY
> Srinivasan
> Sent: Tuesday, September 04, 2012 8:29 PM
> To: Greg KH
> Cc: o...@aepfle.de; tho...@redhat.com; linux-kernel@vger.kernel.org;
> d...@redhat.com; a...@canonical.com; de...@linuxdriverproject.org;
> b...@decadent.org.uk
> Subject: RE: [PATCH V4 05/10] Tools: hv: Add an example script to retrieve 
> dhcp
> state
> 
> 
> 
> > -Original Message-
> > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > Sent: Tuesday, September 04, 2012 8:04 PM
> > To: KY Srinivasan
> > Cc: o...@aepfle.de; tho...@redhat.com; linux-kernel@vger.kernel.org;
> > d...@redhat.com; a...@canonical.com; de...@linuxdriverproject.org;
> > b...@decadent.org.uk
> > Subject: Re: [PATCH V4 05/10] Tools: hv: Add an example script to retrieve 
> > dhcp
> > state
> >
> > On Tue, Sep 04, 2012 at 11:39:12PM +, KY Srinivasan wrote:
> > >
> > >
> > > > -----Original Message-
> > > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > > Sent: Tuesday, September 04, 2012 6:59 PM
> > > > To: KY Srinivasan
> > > > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org;
> > o...@aepfle.de;
> > > > a...@canonical.com; b...@decadent.org.uk; tho...@redhat.com;
> > > > d...@redhat.com
> > > > Subject: Re: [PATCH V4 05/10] Tools: hv: Add an example script to 
> > > > retrieve
> > dhcp
> > > > state
> > > >
> > > > On Tue, Sep 04, 2012 at 02:46:37PM -0700, K. Y. Srinivasan wrote:
> > > > > To keep the KVP daemon code free of distro specific details, we 
> > > > > invoke an
> > > > > external script to retrieve the DHCP state. This is an example script 
> > > > > that
> > > > > was used to test the KVP code. This script has to be implemented in a
> Distro
> > > > > specific fashion. For instance on distros that ship with Network 
> > > > > Manager
> > > > enabled,
> > > > > this script can be based on NM APIs.
> > > > >
> > > > > Signed-off-by: K. Y. Srinivasan 
> > > > > Reviewed-by: Haiyang Zhang 
> > > > > ---
> > > > >  tools/hv/hv_get_dhcp_info.sh |   25 +
> > > > >  1 files changed, 25 insertions(+), 0 deletions(-)
> > > > >  create mode 100755 tools/hv/hv_get_dhcp_info.sh
> > > > >
> > > > > diff --git a/tools/hv/hv_get_dhcp_info.sh
> b/tools/hv/hv_get_dhcp_info.sh
> > > > > new file mode 100755
> > > > > index 000..3de4587
> > > > > --- /dev/null
> > > > > +++ b/tools/hv/hv_get_dhcp_info.sh
> > > > > @@ -0,0 +1,25 @@
> > > > > +#!/bin/bash
> > > > > +
> > > > > +# This example script retrieves the DHCP state of a given interface.
> > > > > +# In the interest of keeping the KVP daemon code free of distro 
> > > > > specific
> > > > > +# information; the kvp daemon code invokes this external script to
> gather
> > > > > +# DHCP setting for the specific interface.
> > > > > +#
> > > > > +# Input: Name of the interface
> > > > > +#
> > > > > +# Output: The script prints the string "Enabled" to stdout to 
> > > > > indicate
> > > > > +#that DHCP is enabled on the interface.
> > > >
> > > > What happens if DHCP is not enabled on the interface?  Shouldn't that
> > > > also return something other than "success"?
> > >
> > > The script is expected to write "Enabled" to stdout to indicate if DHCP is
> > enabled; if this
> > > is not the case, implicitly we assume DHCP is not enabled (since this is 
> > > a binary
> > state).
> >
> > It's not really "binary" given that you are expecting "Enabled" or
> > nothing, right?  "Disabled" would make a bit more sense perhaps?
> 
> In the KVP daemon code, I currently only check for "Enabled" to see if
> DHCP is enabled.
> 
> >
> > Having a script return "nothing at all" to show the failure of a state
> > doesn't seem the wisest thing, does it?
> 
> I can certainly resubmit this patch with the change you are suggesting. This 
> patch
> adds a new file
> and does not impact any of the other patches that follows this. Would you want
> me to resubmit all of the
> remaining patches starting with this one.

I have resent this patch with the change you have suggested. Since this patch 
is a 
standalone patch I have not resent the other patches that follow this patch. 
Let me
know if you want me to resend the remaining patches.

Regards,

K. Y 
> ___
> devel mailing list
> de...@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
> 
> 



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: 3.7 RC1

[KY Srinivasan]

> -Original Message-
> From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> Sent: Tuesday, October 23, 2012 1:47 AM
> To: KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com
> Subject: Re: 3.7 RC1
> 
> On Mon, Oct 22, 2012 at 04:37:45PM -0700, K. Y. Srinivasan wrote:
> >
> > While testing 3.7 RC1 I discovered that invoking the function
> orderly_poweroff()
> > from an interrupt context will trigger an ASSERT(). This was not the case 
> > till
> > recently. The comment preceding the orderly_poweroff() function claims that
> this
> > function can be invoked from any context and in the current Hyper-V util 
> > driver,
> > we support host-driven orderly shut down of the guest by invoking this
> > orderly_poweroff() function in the context of the message callback. This 
> > code
> has
> > been working for a very long time and it is broken now. Is my assumption 
> > that
> > orderly_poweroff() could be invoked from the interrupt context a wrong
> assumption?
> 
> You can't call orderly_poweroff() from interrupt context.

Thanks Dan; I am curious  to understand the basis for your assertion.
As I noted earlier  the documentation for this function clearly says it can
be called from any context. Furthermore, __orderly_poweroff(), the helper
function allocates memory with the GFP_ATOMIC flag set. Lastly, the behavior
of orderly_poweroff() has been such that this function could be called from 
interrupt context
for a very long time and something has changed now. For what it is worth, there 
are other users in
the kernel (in 3.7 RC1) that are invoking the orderly_poweroff() function from 
interrupt
context other than the Hyper-V shutdown handler:  fsl_hv_shutdown_isr() in 
drivers/virt/fsl_hypervisor.c.
I suspect there are other users as well that have made this similar assumption.


Regards,

K. Y




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: 3.7 RC1

[KY Srinivasan]

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Monday, October 22, 2012 7:34 PM
> To: KY Srinivasan
> Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org; 
> o...@aepfle.de;
> a...@canonical.com; jasow...@redhat.com
> Subject: Re: 3.7 RC1
> 
> On Mon, Oct 22, 2012 at 04:37:45PM -0700, K. Y. Srinivasan wrote:
> >
> > While testing 3.7 RC1 I discovered that invoking the function
> orderly_poweroff()
> > from an interrupt context will trigger an ASSERT(). This was not the case 
> > till
> > recently. The comment preceding the orderly_poweroff() function claims that
> this
> > function can be invoked from any context and in the current Hyper-V util 
> > driver,
> > we support host-driven orderly shut down of the guest by invoking this
> > orderly_poweroff() function in the context of the message callback. This 
> > code
> has
> > been working for a very long time and it is broken now. Is my assumption 
> > that
> > orderly_poweroff() could be invoked from the interrupt context a wrong
> assumption?
> 
> Can you check 3.7-rc2?  If that also fails, care to do a 'git bisect'
> from 3.6 to 3.7-rc1 to find the problem patch?

Looks like this problem is fixed in rc2.

Regards,

K. Y


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: Drivers: scsi

[KY Srinivasan]

> -Original Message-
> From: James Bottomley [mailto:james.bottom...@hansenpartnership.com]
> Sent: Wednesday, October 24, 2012 6:25 PM
> To: KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; oher...@suse.com; h...@infradead.org; linux-
> s...@vger.kernel.org
> Subject: Re: Drivers: scsi
> 
> On Wed, 2012-10-24 at 09:25 -0700, K. Y. Srinivasan wrote:
> > When the low level driver returns SCSI_MLQUEUE_DEVICE_BUSY,
> > how is the command retried; I suspect the retry is done after some delay.
> 
> Delay depends mainly on I/O pressure and the unplug timer in the block
> layer.
> 
> > Is this delay programmable? If the device state changes,
> > can the low level driver notify upper layers that it can now handle
> > the command that it had failed earlier with SCSI_MLQUEUE_DEVICE_BUSY.
> 
> In theory, you can call blk_run_queue() from the event handler that sees
> the device become ready.  I don't think any driver actually does this,
> but I can't see it would cause any problem.

I will try this. Thanks for your prompt response.

Regards,

K. Y
 

RE: 3.7 RC1

[KY Srinivasan]

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Monday, October 22, 2012 7:34 PM
> To: KY Srinivasan
> Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org; 
> o...@aepfle.de;
> a...@canonical.com; jasow...@redhat.com
> Subject: Re: 3.7 RC1
> 
> On Mon, Oct 22, 2012 at 04:37:45PM -0700, K. Y. Srinivasan wrote:
> >
> > While testing 3.7 RC1 I discovered that invoking the function
> orderly_poweroff()
> > from an interrupt context will trigger an ASSERT(). This was not the case 
> > till
> > recently. The comment preceding the orderly_poweroff() function claims that
> this
> > function can be invoked from any context and in the current Hyper-V util 
> > driver,
> > we support host-driven orderly shut down of the guest by invoking this
> > orderly_poweroff() function in the context of the message callback. This 
> > code
> has
> > been working for a very long time and it is broken now. Is my assumption 
> > that
> > orderly_poweroff() could be invoked from the interrupt context a wrong
> assumption?
> 
> Can you check 3.7-rc2?  If that also fails, care to do a 'git bisect'
> from 3.6 to 3.7-rc1 to find the problem patch?

The problem still persists in 3.7-rc2. Sorry for the false positive; we had a 
script problem
that indicated that the problem had gone away. Manual testing has shown that 
the problem
persists. We will investigate it further.

Regards,

K. Y
> 
> thanks,
> 
> greg k-h
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 3/3] Drivers: hv: Get rid of hv_ringbuffer_cleanup()

[KY Srinivasan]

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Wednesday, October 24, 2012 6:45 PM
> To: KY Srinivasan
> Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org; 
> o...@aepfle.de;
> a...@canonical.com; jasow...@redhat.com
> Subject: Re: [PATCH 3/3] Drivers: hv: Get rid of hv_ringbuffer_cleanup()
> 
> On Fri, Oct 12, 2012 at 01:22:43PM -0700, K. Y. Srinivasan wrote:
> > hv_ringbuffer_cleanup() is an empty function; get rid of it.
> >
> > Signed-off-by: K. Y. Srinivasan 
> > Reviewed-by: Haiyang Zhang 
> > Reported-by: Jason Wang 
> > Acked-by: Jason Wang 
> > ---
> >  drivers/hv/channel.c  |4 
> >  drivers/hv/hyperv_vmbus.h |1 -
> >  drivers/hv/ring_buffer.c  |   11 ---
> >  3 files changed, 0 insertions(+), 16 deletions(-)
> 
> That's nice, but this patch causes build failures, so I can't accept it.
> 
> Please test your patches better (hint, a simple 'git grep' would have
> worked here...)

Sorry about that. I am pretty sure I built (with this patch) both vmbus and the 
util drivers.
This function is  a private function for the vmbus driver and in fact I went 
back and rebuilt the
drivers in the hv directory without any problem (with this patch applied) in 
the same tree where
I developed these patches (linux-next tree dated October 2, 2012). 
Specifically, what was
the build problem you ran into.

Regards,

K. Y


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/2] mm: Export vm_committed_as

[KY Srinivasan]

> -Original Message-
> From: Andrew Morton [mailto:a...@linux-foundation.org]
> Sent: Tuesday, October 09, 2012 3:48 PM
> To: Greg KH
> Cc: KY Srinivasan; o...@aepfle.de; linux-kernel@vger.kernel.org;
> a...@firstfloor.org; a...@canonical.com; de...@linuxdriverproject.org
> Subject: Re: [PATCH 1/2] mm: Export vm_committed_as
> 
> On Mon, 8 Oct 2012 06:35:39 -0700
> Greg KH  wrote:
> 
> > On Mon, Oct 08, 2012 at 03:35:50AM +, KY Srinivasan wrote:
> > >
> > >
> > > > -Original Message-
> > > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > > Sent: Sunday, October 07, 2012 8:44 PM
> > > > To: KY Srinivasan
> > > > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org;
> o...@aepfle.de;
> > > > a...@canonical.com; a...@linux-foundation.org; a...@firstfloor.org
> > > > Subject: Re: [PATCH 1/2] mm: Export vm_committed_as
> > > >
> > > > On Sun, Oct 07, 2012 at 04:59:45PM -0700, K. Y. Srinivasan wrote:
> > > > > The policy engine on the host expects the guest to report the
> > > > > committed_as. Since this variable is not exported,
> > > > > export this symbol.
> > > >
> > > > Why are these symbols not needed by either Xen or KVM or vmware, which
> > > > I think all support the same thing, right?
> > >
> > > The basic balloon driver does not need this symbol since the basic balloon
> driver
> > > is not automatically driven by the host. On the Windows host we have a 
> > > policy
> engine that
> > > drives the balloon driver based on both guest level memory pressure that 
> > > the
> guest
> > > reports as well as other system level metrics the host maintains. We need 
> > > this
> symbol to
> > > drive the policy engine on the host.
> >
> > Ok, but you're going to have to get the -mm developers to agree that
> > this is ok before I can accept it.
> 
> Well I guess it won't kill us.

Andrew,

I presumed this was an Ack from you with regards to exporting the
symbol. Looks like Greg is waiting to hear from you before he can check
these patches in. Could you provide an explicit Ack.

Regards,

K. Y
 
> 
> I do wonder what relevance vm_committed_as has to processes which are
> running within a memcg container?
> 
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: drivres/hv

[KY Srinivasan]

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Friday, November 02, 2012 7:37 PM
> To: KY Srinivasan
> Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org
> Subject: Re: drivres/hv
> 
> On Fri, Nov 02, 2012 at 03:11:23PM -0700, K. Y. Srinivasan wrote:
> >
> > Greg,
> >
> > Recently, I had  re-sent a few patches. You have applied all the patches 
> > except
> the balloon driver
> > related patches. Should I resend the balloon driver  patches. Let me know.
> 
> I can't apply the balloon driver until you get an ack from the
> maintainers of the code you were exporting the symbols from.
> 
> Get that, and then resend them, as they are long gone from my "to-apply"
> queue.

I was under  the impression Andrew was ok with the export - I have resent the 
original email
from Andrew on this. Let me know if this is enough. I will resend the patches 
once I hear from you.

Regards,

K. Y 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: Drivers: scsi

[KY Srinivasan]

> -Original Message-
> From: James Bottomley [mailto:james.bottom...@hansenpartnership.com]
> Sent: Sunday, November 04, 2012 4:39 AM
> To: KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; oher...@suse.com; h...@infradead.org; linux-
> s...@vger.kernel.org
> Subject: Re: Drivers: scsi
> 
> On Sat, 2012-11-03 at 09:40 -0700, K. Y. Srinivasan wrote:
> > Do we currently support dynamic re-sizing of LUNs. Hyper-V can
> > notify capacity change via sense data and I was wondering if this
> > is handled in the generic scsi code.
> 
> Depends what you mean by "dynamic".  Experience shows that most users
> really don't want all this to happen without manual intervention
> (particularly on shrink).  However, we have the mechanics in place.
> Just send ASC/Q 2A/09 and it can get vectored up to LVM which resizes
> the volume and can be scripted automatically to resize the filesystem.
> 
> hypervisor people tend to have problems with scripting, so there are
> some hacks that do this outside of the normal event system ... see for
> example virtio_scsi.c

Thanks for the prompt response.

Regards,

K. Y
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�&j:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

RE: [PATCH 1/2] mm: Export vm_committed_as

[KY Srinivasan]

> -Original Message-
> From: Andrew Morton [mailto:a...@linux-foundation.org]
> Sent: Monday, November 05, 2012 4:45 PM
> To: KY Srinivasan
> Cc: Greg KH; o...@aepfle.de; linux-kernel@vger.kernel.org; 
> a...@firstfloor.org;
> a...@canonical.com; de...@linuxdriverproject.org; linux...@kvack.org;
> Hiroyuki Kamezawa; Michal Hocko; Johannes Weiner; Ying Han
> Subject: Re: [PATCH 1/2] mm: Export vm_committed_as
> 
> On Sat, 3 Nov 2012 14:09:38 +
> KY Srinivasan  wrote:
> 
> >
> >
> > > >
> > > > Ok, but you're going to have to get the -mm developers to agree that
> > > > this is ok before I can accept it.
> > >
> > > Well I guess it won't kill us.
> >
> > Andrew,
> >
> > I presumed this was an Ack from you with regards to exporting the
> > symbol. Looks like Greg is waiting to hear from you before he can check
> > these patches in. Could you provide an explicit Ack.
> >
> 
> Well, I do have some qualms about exporting vm_committed_as to modules.
> 
> vm_committed_as is a global thing and only really makes sense in a
> non-containerised system.  If the application is running within a
> memory cgroup then vm_enough_memory() and the global overcommit policy
> are at best irrelevant and misleading.
> 
> If use of vm_committed_as is indeed a bad thing, then exporting it to
> modules might increase the amount of badness in the kernel.
> 
> 
> I don't think these qualms are serious enough to stand in the way of
> this patch, but I'd be interested in hearing the memcg developers'
> thoughts on the matter?
> 
> 
> Perhaps you could provide a detailed description of why your module
> actually needs this?  Precisely what information is it looking for
> and why?  If we know that then perhaps a more comfortable alternative
> can be found.

The Hyper-V host has a policy engine for managing available physical memory 
across
competing virtual machines. This policy decision is based on a number of 
parameters
including the memory pressure reported by the guest. Currently, the pressure 
calculation is
based on the memory commitment made by the guest. From what I can tell, the 
ratio of
currently allocated physical memory to the current memory commitment made by 
the guest
(vm_committed_as) is used as one of the parameters in making the memory 
balancing decision on
the host. This is what Windows guests report to the host. So, I need some 
measure of memory
commitments made by the Linux guest. This is the reason I want export 
vm_committed_as. 

Regards,

K. Y  



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 00/17] drivers: hv: kvp

[KY Srinivasan]

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Tuesday, July 24, 2012 11:54 AM
> To: KY Srinivasan
> Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org;
> virtualizat...@lists.osdl.org; o...@aepfle.de; a...@canonical.com;
> net...@vger.kernel.org; b...@decadent.org.uk
> Subject: Re: [PATCH 00/17] drivers: hv: kvp
> 
> On Tue, Jul 24, 2012 at 09:01:12AM -0700, K. Y. Srinivasan wrote:
> > This patchset expands the KVP (Key Value Pair) functionality to
> > implement the mechanism to GET/SET IP addresses in the guest. This
> > functionality is used in Windows Server 2012 to implement VM
> > replication functionality. The way IP configuration information
> > is managed is distro specific. Based on the feedback I have gotten
> > from Olaf, Greg, Steve, Ben and Mairus, I have chosen to seperate
> > distro specific code from this patch-set. Most of the GET operation
> > can be implemented in a way that is completely distro independent and
> > I have implemented that as such and is included in this patch-set.
> > Some of the attributes that can only be fetched in a distro
> > dependent way as well the mechanism for configuring an interface
> > (the SET operation) that is clearly distro specific is to be
> > implemented via external scripts that will be invoked via the KVP
> > code. We define here the interface to these scripts.
> >
> > Adding support for IP injection resulted in some changes to the
> > protocol between the user level daemon and the kernel driver.
> > These changes have been implemented in way that would retain
> > compatibility with older daemons. I would like to thank Olaf and
> > Greg for pointing out the compatibility issue.
> 
> Due to this being the middle of the merge window, I will not be able to
> look at this until after 3.6-rc1 is out.

Thanks Greg. In the meantime, I have addressed all the comments that both Olaf
and Ben have posted on this patch-set. Since addressing these comments changed
some data structures, I think it will be best if you dropped this patch-set. I 
will post the 
updated patch-set shortly.

Regards,

K. Y



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 13/17] Tools: hv: Implement the KVP verb - KVP_OP_SET_IP_INFO

[KY Srinivasan]

> -Original Message-
> From: Olaf Hering [mailto:o...@aepfle.de]
> Sent: Monday, July 30, 2012 2:03 PM
> To: KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; a...@canonical.com; net...@vger.kernel.org;
> b...@decadent.org.uk
> Subject: Re: [PATCH 13/17] Tools: hv: Implement the KVP verb -
> KVP_OP_SET_IP_INFO
> 
> On Tue, Jul 24, K. Y. Srinivasan wrote:
> 
> > +   /*
> > +* Set the configuration for the specified interface with
> > +* the information provided. Since there is no standard
> > +* way to configure an interface, we will have an external
> > +* script that does the job of configuring the interface and
> > +* flushing the configuration.
> > +*
> > +* The parameters passed to this external script are:
> > +* 1. A configuration file that has the specified configuration.
> 
> Maybe this should be written as 'A info file that has the requested
> network configuration' or something like that.

That is the idea. This configuration file simply reflects all the information 
we have
perhaps with some additional constant information. The script is free to ignore 
what
it does not need. 

> 
> > +*
> > +* We will embed the name of the interface in the configuration
> > +* file: ifcfg-ethx (where ethx is the interface name).
> 
> I think the intention here is to use the generated file as is. Depending
> on the distro in the guest the file may need some processing. So I think
> the actual interface name should also be part of the file.

That is not the intention although on some distros, the format of this file
may be closer to the distro specific configuration file than others. I will 
however include the name of the interface in the file as well.
 

> 
> > +*
> > +* Here is the format of the ip configuration file:
> > +*
> > +* HWADDR=macaddr
> > +* BOOTPROTO=dhcp (dhcp enabled for the interface)
> 
> While BOOTPROTO= is used in current network config files, its meaning
> there is unrelated to what its meant here. Here it means DHCP=yes/no, so
> I think the file should contain just that. And as the code is written
> now BOOTPROTO= is optional, which is not mentioned in the comment.

I will fix this.

> 
> > +* NM_CONTROLLED=no (this interface will not be controlled by NM)
> 
> I think this is not up to kvp_deamon to decide what controls the
> interface. Maybe one day NM is sufficiently advanced technology that it
> can cope with such requests?
> The helper script should decide if the NM flag should be written to the
> final config file.

As I noted earlier, the external script can choose to interpret the contents of 
this
file in a way that it makes sense for the distribution. Maybe, I will get rid 
of all the 
constant information.

> 
> > +* PEERDNS=yes
> > +* IPADDR_x=ipaddr
> > +* NETMASK_x=netmask
> > +* GATEWAY_x=gateway
> 
> This should mention that its either IPADDR=ipaddr1 or 'IPADDR=ipaddr1;
> IPADDR_1=ipaddr2'
> 
> > +* DNSx=dns
> 
> This should mention that its either DNS=ipaddr1 or 'DNS=ipaddr1;
> DNS1=ipaddr2'

I take it that you want the comments to be more explicit on the format.

> 
> Olaf
> 
> 

Thank you Olaf,

K. Y

RE: [PATCH 13/17] Tools: hv: Implement the KVP verb - KVP_OP_SET_IP_INFO

[KY Srinivasan]

> -Original Message-
> From: Olaf Hering [mailto:o...@aepfle.de]
> Sent: Monday, July 30, 2012 1:33 PM
> To: KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; a...@canonical.com; net...@vger.kernel.org;
> b...@decadent.org.uk
> Subject: Re: [PATCH 13/17] Tools: hv: Implement the KVP verb -
> KVP_OP_SET_IP_INFO
> 
> On Tue, Jul 24, K. Y. Srinivasan wrote:
> 
> > +static char *kvp_get_if_name(char *guid)
> > +{
> > +   DIR *dir;
> > +   struct dirent *entry;
> > +   FILE*file;
> > +   char*p, *q, *x;
> > +   char*if_name = NULL;
> > +   charbuf[256];
> > +   char *kvp_net_dir = "/sys/class/net/";
> > +   char dev_id[100];
> 
> Perhaps this can be written as char dev_id[100] = "short string";?

Why?

> 
> > +
> > +   dir = opendir(kvp_net_dir);
> > +   if (dir == NULL)
> > +   return NULL;
> > +
> > +   memset(dev_id, 0, sizeof(dev_id));
> > +   strcat(dev_id, kvp_net_dir);
> > +   q = dev_id + strlen(kvp_net_dir);
> > +
> > +   while ((entry = readdir(dir)) != NULL) {
> > +   /*
> > +* Set the state for the next pass.
> > +*/
> > +   *q = '\0';
> > +   strcat(dev_id, entry->d_name);
> > +   strcat(dev_id, "/device/device_id");
> 
> Maybe this (and other) strcat should be changed to snprintf?

Already done.

> 


> > +
> > +   file = fopen(dev_id, "r");
> > +   if (file == NULL)
> > +   continue;
> > +
> > +   p = fgets(buf, sizeof(buf), file);
> > +   if (p) {
> > +   x = strchr(p, '\n');
> > +   if (x)
> > +   *x = '\0';
> > +
> > +   if (!strcmp(p, guid)) {
> > +   /*
> > +* Found the guid match; return the interface
> > +* name. The caller will free the memory.
> > +*/
> > +   if_name = strdup(entry->d_name);
> > +   break;
> 
> This will leave 'file' open.
> I have seen it in some other place as well.

I will audit all instances and fix it.

> 
> > +   }
> > +   }
> > +   fclose(file);
> > +   }
> > +
> > +   closedir(dir);
> > +   return if_name;
> > +}
> > +
> > +/*
> > + * Retrieve the MAC address given the interface name.
> > + */
> > +
> > +static char *kvp_if_name_to_mac(char *if_name)
> > +{
> > +   FILE*file;
> > +   char*p, *x;
> > +   charbuf[256];
> > +   char addr_file[100];
> > +   int i;
> > +   char *mac_addr = NULL;
> > +
> > +   memset(addr_file, 0, sizeof(addr_file));
> > +   strcat(addr_file, "/sys/class/net/");
> > +   strcat(addr_file, if_name);
> > +   strcat(addr_file, "/address");
> 
> snprintf?

Already fixed.

> 
> > +
> > +   file = fopen(addr_file, "r");
> > +   if (file == NULL)
> > +   return NULL;
> > +
> > +   p = fgets(buf, sizeof(buf), file);
> > +   if (p) {
> > +   x = strchr(p, '\n');
> > +   if (x)
> > +   *x = '\0';
> > +   for (i = 0; i < strlen(p); i++)
> > +   p[i] = toupper(p[i]);
> > +   mac_addr = strdup(p);
> > +   }
> > +
> > +   fclose(file);
> > +   return mac_addr;
> > +}
> > +
> > +
> >  static void kvp_process_ipconfig_file(char *cmd,
> > char *config_buf, int len,
> > int element_size, int offset)
> > @@ -800,6 +910,314 @@ getaddr_done:
> >  }
> >
> >
> > +static int expand_ipv6(char *addr, int type)
> > +{
> > +   int ret;
> > +   struct in6_addr v6_addr;
> > +
> > +   ret = inet_pton(AF_INET6, addr, &v6_addr);
> > +
> > +   if (ret != 1) {
> > +   if (type == NETMASK)
> > +   return 1;
> > +   return 0;
> > +   }
> > +
> > +   sprintf(addr, "%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:"
> > +   "%02x%02x:%02x%02x:%02x%02x",
> > +   (int)v6_addr.s6_addr[0], (int)v6_addr.s6_addr[1],

RE: [PATCH 13/17] Tools: hv: Implement the KVP verb - KVP_OP_SET_IP_INFO

[KY Srinivasan]

> -Original Message-
> From: Ben Hutchings [mailto:b...@decadent.org.uk]
> Sent: Monday, July 30, 2012 3:19 PM
> To: KY Srinivasan
> Cc: Olaf Hering; gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; a...@canonical.com; net...@vger.kernel.org
> Subject: Re: [PATCH 13/17] Tools: hv: Implement the KVP verb -
> KVP_OP_SET_IP_INFO
> 
> On Mon, Jul 30, 2012 at 06:32:15PM +, KY Srinivasan wrote:
> >
> >
> > > -Original Message-
> > > From: Olaf Hering [mailto:o...@aepfle.de]
> > > Sent: Monday, July 30, 2012 2:03 PM
> > > To: KY Srinivasan
> > > Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> > > de...@linuxdriverproject.org; a...@canonical.com; net...@vger.kernel.org;
> > > b...@decadent.org.uk
> > > Subject: Re: [PATCH 13/17] Tools: hv: Implement the KVP verb -
> > > KVP_OP_SET_IP_INFO
> > >
> > > On Tue, Jul 24, K. Y. Srinivasan wrote:
> > >
> > > > +   /*
> > > > +* Set the configuration for the specified interface with
> > > > +* the information provided. Since there is no standard
> > > > +* way to configure an interface, we will have an external
> > > > +* script that does the job of configuring the interface and
> > > > +* flushing the configuration.
> > > > +*
> > > > +* The parameters passed to this external script are:
> > > > +* 1. A configuration file that has the specified configuration.
> > >
> > > Maybe this should be written as 'A info file that has the requested
> > > network configuration' or something like that.
> >
> > That is the idea. This configuration file simply reflects all the
> > information we have perhaps with some additional constant
> > information. The script is free to ignore what it does not need.
> [...]
> 
> This does not strike me as a sensible interface.  If scripts are
> 'free to ignore' information then the KVP interface becomes unreliable
> as a means for managing networking on Linux guests.  I would suggest
> that at the least the script should be able to report that it did not
> recognise some parts of the configuration.  This would be logged
> and/or reported back to the hypervisor.
> 
> (This is separate from the issue of constant configuration lines;
> for some distributions the script might recognise but ignore them
> because they have no use on that distribution.  I don't see the
> point in constant lines, but they don't seem to result in any
> unreliability.)

Ben,

I see your point. I have cleaned up the contents of the KVP produced
configuration file to not include constant information that can be
auto generated by the distro specific script if it needs to. Also, I have
tried to make the documentation of the contents of the file a little
better. I will send out these new patches soon. Still, there is a possibility 
that
some of the content of this file may be redundant on a specific distro and I 
think
that should be fine. For instance, per Olaf's suggestion, I have included a 
line that
specifies the interface name in the file (in addition to the mac address). 
Given the
current format of the name of the config file (where the interface name is 
embedded
in the config file name, this additional name entry may be redundant on some 
distros.

Once again, thank you for taking the time to review this code.

Regards,

K. Y 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/1] Drivers: hv: vmbus: Fix a bug in the handling of channel offers

[KY Srinivasan]

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Monday, August 26, 2013 2:57 PM
> To: KY Srinivasan
> Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org; 
> o...@aepfle.de;
> a...@canonical.com; jasow...@redhat.com; sta...@vger.kernel.org
> Subject: Re: [PATCH 1/1] Drivers: hv: vmbus: Fix a bug in the handling of 
> channel
> offers
> 
> On Mon, Aug 26, 2013 at 02:08:58PM -0700, K. Y. Srinivasan wrote:
> > The channel state should be correctly set before registering the device. In 
> > the
> current
> > code the driver probe would fail for channels that have been rescinded and
> subsequently
> > re-offered. Fix the bug.
> >
> > Signed-off-by: K. Y. Srinivasan 
> > Cc: 
> 
> What older kernel versions have this bug?  When did it show up?

The patch that introduced the bug is:

commit e68d2971d26577b932a16333ce165af98a96e068
Author: K. Y. Srinivasan 
Date:   Thu May 23 12:02:32 2013 -0700

Drivers: hv: vmbus: Implement multi-channel support

I think 3.10 shipped with this bug.

Regards,

K. Y
> 
> thanks,
> 
> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/1] Drivers: hv: vmbus: Fix a bug in the handling of channel offers

[KY Srinivasan]

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Monday, August 26, 2013 4:14 PM
> To: KY Srinivasan
> Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org; 
> o...@aepfle.de;
> a...@canonical.com; jasow...@redhat.com; sta...@vger.kernel.org
> Subject: Re: [PATCH 1/1] Drivers: hv: vmbus: Fix a bug in the handling of 
> channel
> offers
> 
> On Mon, Aug 26, 2013 at 10:48:16PM +, KY Srinivasan wrote:
> >
> >
> > > -Original Message-
> > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > Sent: Monday, August 26, 2013 2:57 PM
> > > To: KY Srinivasan
> > > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org;
> o...@aepfle.de;
> > > a...@canonical.com; jasow...@redhat.com; sta...@vger.kernel.org
> > > Subject: Re: [PATCH 1/1] Drivers: hv: vmbus: Fix a bug in the handling of
> channel
> > > offers
> > >
> > > On Mon, Aug 26, 2013 at 02:08:58PM -0700, K. Y. Srinivasan wrote:
> > > > The channel state should be correctly set before registering the 
> > > > device. In
> the
> > > current
> > > > code the driver probe would fail for channels that have been rescinded 
> > > > and
> > > subsequently
> > > > re-offered. Fix the bug.
> > > >
> > > > Signed-off-by: K. Y. Srinivasan 
> > > > Cc: 
> > >
> > > What older kernel versions have this bug?  When did it show up?
> >
> > The patch that introduced the bug is:
> >
> > commit e68d2971d26577b932a16333ce165af98a96e068
> > Author: K. Y. Srinivasan 
> > Date:   Thu May 23 12:02:32 2013 -0700
> >
> > Drivers: hv: vmbus: Implement multi-channel support
> >
> > I think 3.10 shipped with this bug.
> 
> Why do you think that, it's not what git shows:
>   $ git describe --contains e68d2971d26577b932a16333ce165af98a96e068
>   v3.11-rc1~157^2~64
> 
> So this is a 3.11-only thing, right?

My mistake; I was looking at some version of linux-3.10 code that had this 
problem and so I assumed it must have shipped
in 3.10.

Regards,

K. Y
> 
> thanks,
> 
> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/1] X86: Hyper-V: Get the local APIC timer frequency from the hypervisor

[KY Srinivasan]

> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: Tuesday, August 27, 2013 12:26 AM
> To: KY Srinivasan
> Cc: o...@aepfle.de; a...@canonical.com; x...@kernel.org;
> de...@linuxdriverproject.org; gre...@linuxfoundation.org;
> jasow...@redhat.com; linux-kernel@vger.kernel.org; h...@zytor.com
> Subject: Re: [PATCH 1/1] X86: Hyper-V: Get the local APIC timer frequency from
> the hypervisor
> 
> >>> On 27.08.13 at 01:42, "K. Y. Srinivasan"  wrote:
> > Hyper-V supports a mechanism for retrieving the local API frequency.
> 
> "APIC"?
> 
> > @@ -27,6 +27,13 @@
> >  #define HV_X64_MSR_VP_RUNTIME_AVAILABLE(1 << 0)
> >  /* Partition Reference Counter (HV_X64_MSR_TIME_REF_COUNT) available*/
> >  #define HV_X64_MSR_TIME_REF_COUNT_AVAILABLE(1 << 1)
> > +
> > +/* Local APIC timer frequency MSR (HV_X64_MSR_APIC_FREQUENCY) is
> available */
> > +#define HV_X64_MSR_APIC_FREQUENCY_AVAILABLE (1 << 11)
> > +
> > +/* TSC frequency MSR (HV_X64_MSR_TSC_FREQUENCY) is available */
> > +#define HV_X64_MSR_TSC_FREQUENCY_AVAILABLE (1 << 11)
> 
> Are these two really the same bit? If so, why two different names?
> 
> > @@ -136,6 +143,12 @@
> >  /* MSR used to read the per-partition time reference counter */
> >  #define HV_X64_MSR_TIME_REF_COUNT  0x4020
> >
> > +/* MSR used to retrive the TSC frequency */
> > +#define HV_X64_MSR_TSC_FREQUENCY   0x4022
> > +
> > +/* MSR used to retrive the local APIC timer frequency */
> > +#define HV_X64_MSR_APIC_FREQUENCY  0x4023
> 
> "retrieve" (twice)?
> 
> > @@ -76,6 +77,22 @@ static void __init ms_hyperv_init_platform(void)
> > printk(KERN_INFO "HyperV: features 0x%x, hints 0x%x\n",
> >ms_hyperv.features, ms_hyperv.hints);
> >
> > +   if (ms_hyperv.features & HV_X64_MSR_APIC_FREQUENCY_AVAILABLE) {
> > +   /*
> > +* There is no need to calibrate APIC timer frequency;
> > +* nor is there a need to calibrate timer.
> > +*/
> > +   legacy_pic = &null_legacy_pic;
> 
> This clearly disables more than just the calibration, so the comment
> may be misleading to future readers.
> 
> > +
> > +   /*
> > +* Get the APIC frequency.
> > +*/
> > +   rdmsrl(HV_X64_MSR_APIC_FREQUENCY,
> lapic_timer_frequency);
> > +   lapic_timer_frequency /= HZ;
> 
> Is this safe? I.e. are the high 32 bits of the MSR guaranteed to
> be zero, now and forever?
> 
> > +   printk(KERN_INFO "HyperV: LAPIC Timer Frequency: 0x%x\n",
> > +   lapic_timer_frequency);
> 
> As a minor note, I generally recommend using %#x as being one
> byte shorter than the spelled out 0x%x.
> 
> Jan

Thanks Jan. I will fix the issues you have pointed out and resubmit the patch.

Regards,

K. Y
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/2] hyperv-fb: add pci stub

[KY Srinivasan]

> -Original Message-
> From: Gerd Hoffmann [mailto:kra...@redhat.com]
> Sent: Wednesday, October 02, 2013 4:55 AM
> Cc: Gerd Hoffmann; KY Srinivasan; Haiyang Zhang; Jean-Christophe Plagniol-
> Villard; Tomi Valkeinen; open list:Hyper-V CORE AND...; open list:FRAMEBUFFER
> LAYER; open list
> Subject: [PATCH 1/2] hyperv-fb: add pci stub
> 
> This patch adds a pci stub driver to hyper-fb.  The hyperv framebuffer
> driver will bind to the pci device then, so linux kernel and userspace
> know there is a proper kernel driver for the device active.  lspci shows
> this for example:
> 
> [root@dhcp231 ~]# lspci -vs8
> 00:08.0 VGA compatible controller: Microsoft Corporation Hyper-V virtual
> VGA (prog-if 00 [VGA controller])
> Flags: bus master, fast devsel, latency 0, IRQ 11
> Memory at f800 (32-bit, non-prefetchable) [size=64M]
> Expansion ROM at  [disabled]
> Kernel driver in use: hyperv_fb
> 
> Another effect is that the xorg vesa driver will not attach to the
> device and thus the Xorg server will automatically use the fbdev
> driver instead.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  drivers/video/hyperv_fb.c | 40
> +++-
>  1 file changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/video/hyperv_fb.c b/drivers/video/hyperv_fb.c
> index 8ac99b8..8d456dc 100644
> --- a/drivers/video/hyperv_fb.c
> +++ b/drivers/video/hyperv_fb.c
> @@ -795,12 +795,21 @@ static int hvfb_remove(struct hv_device *hdev)
>  }
> 
> 
> +static DEFINE_PCI_DEVICE_TABLE(pci_stub_id_table) = {
> + {
> + .vendor  = PCI_VENDOR_ID_MICROSOFT,
> + .device  = PCI_DEVICE_ID_HYPERV_VIDEO,
> + },
> + { /* end of list */ }
> +};
> +
>  static const struct hv_vmbus_device_id id_table[] = {
>   /* Synthetic Video Device GUID */
>   {HV_SYNTHVID_GUID},
>   {}
>  };
> 
> +MODULE_DEVICE_TABLE(pci, pci_stub_id_table);
>  MODULE_DEVICE_TABLE(vmbus, id_table);
> 
>  static struct hv_driver hvfb_drv = {
> @@ -810,14 +819,43 @@ static struct hv_driver hvfb_drv = {
>   .remove = hvfb_remove,
>  };
> 
> +static int hvfb_pci_stub_probe(struct pci_dev *pdev,
> +const struct pci_device_id *ent)
> +{
> + return 0;
> +}
> +
> +static void hvfb_pci_stub_remove(struct pci_dev *pdev)
> +{
> +}
> +
> +static struct pci_driver hvfb_pci_stub_driver = {
> + .name = KBUILD_MODNAME,
> + .id_table = pci_stub_id_table,
> + .probe =hvfb_pci_stub_probe,
> + .remove =   hvfb_pci_stub_remove,
> +};
> 
>  static int __init hvfb_drv_init(void)
>  {
> - return vmbus_driver_register(&hvfb_drv);
> + int ret;
> +
> + ret = vmbus_driver_register(&hvfb_drv);
> + if (ret != 0)
> + return ret;
> +
> + ret = pci_register_driver(&hvfb_pci_stub_driver);
> + if (ret != 0) {
> + vmbus_driver_unregister(&hvfb_drv);
> + return ret;
> + }
> +
> + return 0;
>  }
> 
>  static void __exit hvfb_drv_exit(void)
>  {
> + pci_unregister_driver(&hvfb_pci_stub_driver);
>   vmbus_driver_unregister(&hvfb_drv);
>  }
> 
> --
> 1.8.3.1
Gerd,

Thanks for doing this. This certainly will address some of the issues that are 
reported. I do have a question though - how would this work if we don't have 
PCI bus in the guest.

Regards,

K. Y
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: Drivers: scsi: FLUSH timeout

[KY Srinivasan]

> -Original Message-
> From: geert.uytterhoe...@gmail.com [mailto:geert.uytterhoe...@gmail.com]
> On Behalf Of Geert Uytterhoeven
> Sent: Wednesday, September 25, 2013 1:40 AM
> To: KY Srinivasan
> Cc: Mike Christie; Jack Wang; Greg KH; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; oher...@suse.com; jbottom...@parallels.com;
> h...@infradead.org; linux-s...@vger.kernel.org
> Subject: Re: Drivers: scsi: FLUSH timeout
> 
> On Tue, Sep 24, 2013 at 11:53 PM, KY Srinivasan  wrote:
> > I am not sure how that magic number was arrived at (the 60HZ number). We
> want this to be little higher -
> 
> "60 * HZ" means "60 seconds".
> 
> > would there be any issues raising this to say  180 seconds. This is the 
> > value we
> currently have for I/O
> > timeout.
> 
> So you want to replace it by "180 * HZ", which is ... another magic number?

Ideally, I want this to be adjustable like the way we can change the I/O 
timeout.
Since that has been attempted earlier and rejected (not clear what the reasons 
were),
I was suggesting that we pick a larger number. James, let me know how I should 
proceed here.

Regards,

K. Y
> 
> Gr{oetje,eeting}s,
> 
> Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�&j:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

RE: [PATCH V2 1/1] X86: Hyper-V: Get the local APIC timer frequency from the hypervisor

[KY Srinivasan]

> -Original Message-
> From: H. Peter Anvin [mailto:h...@zytor.com]
> Sent: Monday, September 23, 2013 8:50 AM
> To: KY Srinivasan
> Cc: Gleb Natapov; x...@kernel.org; gre...@linuxfoundation.org; linux-
> ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de;
> a...@canonical.com; jasow...@redhat.com; t...@linutronix.de;
> jbeul...@suse.com; b...@alien8.de; dmitry.torok...@gmail.com
> Subject: Re: [PATCH V2 1/1] X86: Hyper-V: Get the local APIC timer frequency
> from the hypervisor
> 
> On 09/23/2013 06:02 AM, KY Srinivasan wrote:
> >
> > Peter,
> > Please let me know if there are other issues I need to address here. Now 
> > that
> Dmitry has queued up the
> > Hyper-V keyboard driver, with this patch we can fully support Linux on our 
> > uefi
> firmware.
> >
> 
> I just got back from Plumber's -- this is on my short list of things to
> look at.  If you haven't heard anything by Wednesday, ping me.

Ping.

K. Y
> 
>   -hpa
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: Drivers: scsi: FLUSH timeout

[KY Srinivasan]

> -Original Message-
> From: Nicholas A. Bellinger [mailto:n...@linux-iscsi.org]
> Sent: Thursday, October 03, 2013 5:09 AM
> To: KY Srinivasan
> Cc: Geert Uytterhoeven; Mike Christie; Jack Wang; Greg KH; linux-
> ker...@vger.kernel.org; de...@linuxdriverproject.org; oher...@suse.com;
> jbottom...@parallels.com; h...@infradead.org; linux-s...@vger.kernel.org
> Subject: Re: Drivers: scsi: FLUSH timeout
> 
> On Wed, 2013-10-02 at 18:29 +, KY Srinivasan wrote:
> >
> > > -Original Message-
> > > From: geert.uytterhoe...@gmail.com
> [mailto:geert.uytterhoe...@gmail.com]
> > > On Behalf Of Geert Uytterhoeven
> > > Sent: Wednesday, September 25, 2013 1:40 AM
> > > To: KY Srinivasan
> > > Cc: Mike Christie; Jack Wang; Greg KH; linux-kernel@vger.kernel.org;
> > > de...@linuxdriverproject.org; oher...@suse.com;
> jbottom...@parallels.com;
> > > h...@infradead.org; linux-s...@vger.kernel.org
> > > Subject: Re: Drivers: scsi: FLUSH timeout
> > >
> > > On Tue, Sep 24, 2013 at 11:53 PM, KY Srinivasan  
> > > wrote:
> > > > I am not sure how that magic number was arrived at (the 60HZ number). We
> > > want this to be little higher -
> > >
> > > "60 * HZ" means "60 seconds".
> > >
> > > > would there be any issues raising this to say  180 seconds. This is the 
> > > > value
> we
> > > currently have for I/O
> > > > timeout.
> > >
> > > So you want to replace it by "180 * HZ", which is ... another magic 
> > > number?
> >
> > Ideally, I want this to be adjustable like the way we can change the I/O 
> > timeout.
> > Since that has been attempted earlier and rejected (not clear what the 
> > reasons
> were),
> > I was suggesting that we pick a larger number. James, let me know how I 
> > should
> proceed here.
> >
> 
> I think the objection was to making a module parameter for doing this
> globally for all struct scsi_disk, and not the idea of making it
> adjustable on an individual basis per-say..
> 
> What about adding a /sys/class/scsi_disk/$HCTL/flush_timeout..?
> 
> Here's a quick (untested) patch to that effect.

Thanks Nicholas. This is precisely what I was hoping we could agree on. 
James, if this is acceptable, we will go ahead and test this patch.


Regards,

K. Y
> 
> --nab
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index e62d17d..f7a8c5b 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -460,6 +460,38 @@ max_write_same_blocks_store(struct device *dev,
> struct device_attribute *attr,
>  }
>  static DEVICE_ATTR_RW(max_write_same_blocks);
> 
> +static ssize_t
> +flush_timeout_show(struct device *dev,
> +struct device_attribute *attr, char *buf)
> +{
> + struct scsi_disk *sdkp = to_scsi_disk(dev);
> +
> + return snprintf(buf, 20, "%u\n", sdkp->flush_timeout);
> +}
> +
> +static ssize_t
> +flush_timeout_store(struct device *dev,
> + struct device_attribute *attr, const char *buf,
> + size_t count)
> +{
> + struct scsi_disk *sdkp = to_scsi_disk(dev);
> + unsigned int flush_timeout;
> + int err;
> +
> + if (!capable(CAP_SYS_ADMIN))
> + return -EACCES;
> +
> + err = kstrtouint(buf, 10, &flush_timeout);
> +
> + if (flush_timeout > SD_FLUSH_TIMEOUT_MAX)
> + return -EINVAL;
> +
> + sdkp->flush_timeout = flush_timeout;
> +
> + return err ? err : count;
> +}
> +static DEVICE_ATTR_RW(flush_timeout);
> +
>  static struct attribute *sd_disk_attrs[] = {
>   &dev_attr_cache_type.attr,
>   &dev_attr_FUA.attr,
> @@ -472,6 +504,7 @@ static struct attribute *sd_disk_attrs[] = {
>   &dev_attr_provisioning_mode.attr,
>   &dev_attr_max_write_same_blocks.attr,
>   &dev_attr_max_medium_access_timeouts.attr,
> + &dev_attr_flush_timeout.attr,
>   NULL,
>  };
>  ATTRIBUTE_GROUPS(sd_disk);
> @@ -829,7 +862,9 @@ static int sd_setup_write_same_cmnd(struct scsi_device
> *sdp, struct request *rq)
> 
>  static int scsi_setup_flush_cmnd(struct scsi_device *sdp, struct request *rq)
>  {
> - rq->timeout = SD_FLUSH_TIMEOUT;
> + struct scsi_disk *sdkp = scsi_disk(rq->rq_disk);
> +
> + rq->timeout = sdkp->flush_timeout;
>   rq->retries = SD_MAX_RETRIES;
>   rq->cmd[0] = SYNCHRONIZE_CACHE;
>   rq->cmd_len = 10;
> @@ -2841,6 +2876,7 @@ static void sd_probe_async(void *data, async_cookie_t
> co

RE: Drivers: scsi: FLUSH timeout

[KY Srinivasan]

> -Original Message-
> From: Eric Seppanen [mailto:e...@purestorage.com]
> Sent: Thursday, October 03, 2013 1:49 PM
> To: Nicholas A. Bellinger
> Cc: KY Srinivasan; linux-kernel@vger.kernel.org; de...@linuxdriverproject.org;
> linux-s...@vger.kernel.org
> Subject: Re: Drivers: scsi: FLUSH timeout
> 
> On Thu, Oct 3, 2013 at 5:09 AM, Nicholas A. Bellinger
>  wrote:
> >
> > On Wed, 2013-10-02 at 18:29 +, KY Srinivasan wrote:
> > > Ideally, I want this to be adjustable like the way we can change the I/O
> timeout.
> > > Since that has been attempted earlier and rejected (not clear what the
> reasons were),
> > > I was suggesting that we pick a larger number. James, let me know how I
> should proceed here.
> > >
> >
> > I think the objection was to making a module parameter for doing this
> > globally for all struct scsi_disk, and not the idea of making it
> > adjustable on an individual basis per-say..
> >
> > What about adding a /sys/class/scsi_disk/$HCTL/flush_timeout..?
> 
> Do I/O timeouts and flush timeouts need to be independently adjusted?
> If you're having trouble with slow operations, it seems likely to be
> across the board.
> 
> Flush timeout could be defined as 2x the read/write timeout.  Any
> other command-specific timeouts could be scaled the same way.

I like this idea and would result in minimal changes. James, if it ok with you,
I could send you the patch.

Regards,

K. Y
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/1] Drivers: scsi: Derive the FLUSH_TIMEOUT from the basic I/O timeout

[KY Srinivasan]

> -Original Message-
> From: James Bottomley [mailto:jbottom...@parallels.com]
> Sent: Friday, October 04, 2013 2:42 PM
> To: KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; oher...@suse.com; h...@infradead.org;
> e...@purestorage.com; n...@linux-iscsi.org; linux-s...@vger.kernel.org
> Subject: Re: [PATCH 1/1] Drivers: scsi: Derive the FLUSH_TIMEOUT from the 
> basic
> I/O timeout
> 
> On Fri, 2013-10-04 at 12:38 -0700, K. Y. Srinivasan wrote:
> > Rather than having a separate constant for specifying the timeout on FLUSH
> > operations, use the basic I/O timeout value that is already configurable
> > on a per target basis to derive the FLUSH timeout. Looking at the current
> > definitions of these timeout values, the FLUSH operation is supposed to have
> > a value that is twice the normal timeout value. This patch preserves this
> > relationship while leveraging the flexibility of specifying the I/O timeout.
> >
> > I would like to thank Eric Seppanen  and
> > Nicholas A. Bellinger  for their help in resolving
> > this issue.
> >
> > Signed-off-by: K. Y. Srinivasan 
> > ---
> >  drivers/scsi/sd.c |5 +++--
> >  1 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> > index e62d17d..8aff306 100644
> > --- a/drivers/scsi/sd.c
> > +++ b/drivers/scsi/sd.c
> > @@ -829,7 +829,7 @@ static int sd_setup_write_same_cmnd(struct
> scsi_device *sdp, struct request *rq)
> >
> >  static int scsi_setup_flush_cmnd(struct scsi_device *sdp, struct request 
> > *rq)
> >  {
> > -   rq->timeout = SD_FLUSH_TIMEOUT;
> > +   rq->timeout *= 2;
> > rq->retries = SD_MAX_RETRIES;
> > rq->cmd[0] = SYNCHRONIZE_CACHE;
> > rq->cmd_len = 10;
> > @@ -1433,6 +1433,7 @@ static int sd_sync_cache(struct scsi_disk *sdkp)
> >  {
> > int retries, res;
> > struct scsi_device *sdp = sdkp->device;
> > +   unsigned int timeout = sdp->request_queue->rq_timeout;
> 
> The timeout is signed in the function prototype
> 
> > struct scsi_sense_hdr sshdr;
> >
> > if (!scsi_device_online(sdp))
> > @@ -1448,7 +1449,7 @@ static int sd_sync_cache(struct scsi_disk *sdkp)
> >  * flush everything.
> >  */
> > res = scsi_execute_req_flags(sdp, cmd, DMA_NONE, NULL, 0,
> > -&sshdr, SD_FLUSH_TIMEOUT,
> > +&sshdr, timeout * 2,
> >  SD_MAX_RETRIES, NULL, REQ_PM);
> > if (res == 0)
> > break;
> 
> Not like this, please: you now leave us with a dangling #define whose
> name makes you think it should be related to flushing and a couple of
> curious magic constants.  It's almost hand crafted to confuse people
> reading the code.
> 
> Please do it like this instead: with a comment explaining what we're
> doing above the #define and a name that clearly relates to the actions.
> 
> James
> 
> ---
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index e62d17d..5c9496d 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -829,7 +829,7 @@ static int sd_setup_write_same_cmnd(struct scsi_device
> *sdp, struct request *rq)
> 
>  static int scsi_setup_flush_cmnd(struct scsi_device *sdp, struct request *rq)
>  {
> - rq->timeout = SD_FLUSH_TIMEOUT;
> + rq->timeout *= SD_FLUSH_TIMEOUT_MULTIPLIER;
>   rq->retries = SD_MAX_RETRIES;
>   rq->cmd[0] = SYNCHRONIZE_CACHE;
>   rq->cmd_len = 10;
> @@ -1433,6 +1433,8 @@ static int sd_sync_cache(struct scsi_disk *sdkp)
>  {
>   int retries, res;
>   struct scsi_device *sdp = sdkp->device;
> + const int timeout = sdp->request_queue->rq_timeout
> + * SD_FLUSH_TIMEOUT_MULTIPLIER;
>   struct scsi_sense_hdr sshdr;
> 
>   if (!scsi_device_online(sdp))
> @@ -1448,8 +1450,8 @@ static int sd_sync_cache(struct scsi_disk *sdkp)
>* flush everything.
>*/
>   res = scsi_execute_req_flags(sdp, cmd, DMA_NONE, NULL, 0,
> -  &sshdr, SD_FLUSH_TIMEOUT,
> -  SD_MAX_RETRIES, NULL, REQ_PM);
> +  &sshdr, timeout, SD_MAX_RETRIES,
> +  NULL, REQ_PM);
>   if (res == 0)
>   break;
>   }

RE: [PATCH 1/1] Drivers: scsi: Derive the FLUSH_TIMEOUT from the basic I/O timeout

[KY Srinivasan]

> -Original Message-
> From: James Bottomley [mailto:jbottom...@parallels.com]
> Sent: Friday, October 04, 2013 3:31 PM
> To: KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; oher...@suse.com; h...@infradead.org;
> e...@purestorage.com; n...@linux-iscsi.org; linux-s...@vger.kernel.org
> Subject: Re: [PATCH 1/1] Drivers: scsi: Derive the FLUSH_TIMEOUT from the 
> basic
> I/O timeout
> 
> On Fri, 2013-10-04 at 22:01 +, KY Srinivasan wrote:
> >
> > > -Original Message-
> > > From: James Bottomley [mailto:jbottom...@parallels.com]
> > > Sent: Friday, October 04, 2013 2:42 PM
> > > To: KY Srinivasan
> > > Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> > > de...@linuxdriverproject.org; oher...@suse.com; h...@infradead.org;
> > > e...@purestorage.com; n...@linux-iscsi.org; linux-s...@vger.kernel.org
> > > Subject: Re: [PATCH 1/1] Drivers: scsi: Derive the FLUSH_TIMEOUT from the
> basic
> > > I/O timeout
> > >
> > > On Fri, 2013-10-04 at 12:38 -0700, K. Y. Srinivasan wrote:
> > > > Rather than having a separate constant for specifying the timeout on 
> > > > FLUSH
> > > > operations, use the basic I/O timeout value that is already configurable
> > > > on a per target basis to derive the FLUSH timeout. Looking at the 
> > > > current
> > > > definitions of these timeout values, the FLUSH operation is supposed to
> have
> > > > a value that is twice the normal timeout value. This patch preserves 
> > > > this
> > > > relationship while leveraging the flexibility of specifying the I/O 
> > > > timeout.
> > > >
> > > > I would like to thank Eric Seppanen  and
> > > > Nicholas A. Bellinger  for their help in resolving
> > > > this issue.
> > > >
> > > > Signed-off-by: K. Y. Srinivasan 
> > > > ---
> > > >  drivers/scsi/sd.c |5 +++--
> > > >  1 files changed, 3 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> > > > index e62d17d..8aff306 100644
> > > > --- a/drivers/scsi/sd.c
> > > > +++ b/drivers/scsi/sd.c
> > > > @@ -829,7 +829,7 @@ static int sd_setup_write_same_cmnd(struct
> > > scsi_device *sdp, struct request *rq)
> > > >
> > > >  static int scsi_setup_flush_cmnd(struct scsi_device *sdp, struct 
> > > > request
> *rq)
> > > >  {
> > > > -   rq->timeout = SD_FLUSH_TIMEOUT;
> > > > +   rq->timeout *= 2;
> > > > rq->retries = SD_MAX_RETRIES;
> > > > rq->cmd[0] = SYNCHRONIZE_CACHE;
> > > > rq->cmd_len = 10;
> > > > @@ -1433,6 +1433,7 @@ static int sd_sync_cache(struct scsi_disk *sdkp)
> > > >  {
> > > > int retries, res;
> > > > struct scsi_device *sdp = sdkp->device;
> > > > +   unsigned int timeout = sdp->request_queue->rq_timeout;
> > >
> > > The timeout is signed in the function prototype
> > >
> > > > struct scsi_sense_hdr sshdr;
> > > >
> > > > if (!scsi_device_online(sdp))
> > > > @@ -1448,7 +1449,7 @@ static int sd_sync_cache(struct scsi_disk *sdkp)
> > > >  * flush everything.
> > > >  */
> > > > res = scsi_execute_req_flags(sdp, cmd, DMA_NONE, NULL, 
> > > > 0,
> > > > -&sshdr, SD_FLUSH_TIMEOUT,
> > > > +&sshdr, timeout * 2,
> > > >  SD_MAX_RETRIES, NULL, 
> > > > REQ_PM);
> > > > if (res == 0)
> > > > break;
> > >
> > > Not like this, please: you now leave us with a dangling #define whose
> > > name makes you think it should be related to flushing and a couple of
> > > curious magic constants.  It's almost hand crafted to confuse people
> > > reading the code.
> > >
> > > Please do it like this instead: with a comment explaining what we're
> > > doing above the #define and a name that clearly relates to the actions.
> > >
> > > James
> > >
> > > ---
> > >
> > > diff --git a/drivers/scsi/sd.c b/d

RE: [PATCH 1/2] Drivers: hv: balloon: Fix a bug in the hot-add code

[KY Srinivasan]

> -Original Message-
> From: Ben Hutchings [mailto:b...@decadent.org.uk]
> Sent: Friday, July 12, 2013 4:17 PM
> To: KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com; Stable
> Subject: Re: [PATCH 1/2] Drivers: hv: balloon: Fix a bug in the hot-add code
> 
> On Fri, Jul 12, 2013 at 06:56:14AM -0700, K. Y. Srinivasan wrote:
> > As we hot-add 128 MB chunks of memory, we wait to ensure that the memory
> > is onlined before attempting to hot-add the next chunk. If the udev rule for
> > memory hot-add is not executed within the allowed time, we would rollback
> the
> > state and abort further hot-add. Since the hot-add has succeeded and the 
> > only
> > failure is that the memory is not onlined within the allowed time, we 
> > should not
> > be rolling back the state. Fix this bug.
> [...]
> > /*
> >  * Wait for the memory block to be onlined.
> >  */
> > -   t = wait_for_completion_timeout(&dm_device.ol_waitevent,
> 5*HZ);
> > -   if (t == 0) {
> > -   pr_info("hot_add memory timedout\n");
> > -   has->ha_end_pfn -= HA_CHUNK;
> > -   has->covered_end_pfn -=  processed_pfn;
> > -   break;
> > -   }
> > +   wait_for_completion_timeout(&dm_device.ol_waitevent,
> 5*HZ);
> >
> > }
> >
> 
> Well now it might look like a bug that you don't test the result
> of wait_for_completion_timeout().  Maybe update the comment to
> explain why it's OK to continue anyway?

I put in the comment in the patch explaining why it is ok to continue. To 
reiterate,
it is ok to continue because hot add has succeeded. More importantly, what I was
doing earlier - rolling back the state when in fact hot add had succeeded was 
incorrect.

Regards,

K. Y
> 
> Ben.
> 
> --
> Ben Hutchings
> We get into the habit of living before acquiring the habit of thinking.
>   - Albert Camus
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/2] Drivers: hv: balloon: Fix a bug in the hot-add code

[KY Srinivasan]

> -Original Message-
> From: Ben Hutchings [mailto:b...@decadent.org.uk]
> Sent: Friday, July 12, 2013 5:13 PM
> To: KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com; Stable
> Subject: Re: [PATCH 1/2] Drivers: hv: balloon: Fix a bug in the hot-add code
> 
> On Fri, Jul 12, 2013 at 09:07:19PM +, KY Srinivasan wrote:
> [...]
> > > Well now it might look like a bug that you don't test the result
> > > of wait_for_completion_timeout().  Maybe update the comment to
> > > explain why it's OK to continue anyway?
> >
> > I put in the comment in the patch explaining why it is ok to continue.
> [...]
> 
> But that is not nearly as easy to see as the comment that is
> already *in the code* which your patch isn't updating.

Agreed; I will resend the patch with comments added.

Thanks,

K. Y
> 
> Ben.
> 
> --
> Ben Hutchings
> We get into the habit of living before acquiring the habit of thinking.
>   - Albert Camus
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 00/13] hv: clean up dev_attr usage

[KY Srinivasan]

> -Original Message-
> From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org]
> Sent: Friday, September 13, 2013 11:33 AM
> To: KY Srinivasan; Haiyang Zhang
> Cc: de...@linuxdriverproject.org; linux-kernel@vger.kernel.org
> Subject: [PATCH 00/13] hv: clean up dev_attr usage
> 
> Hi,
> 
> Here's a set of 13 patches to get rid of the dev_attrs use in the hv bus
> code, as it will be going away soon.  It's _way_ bigger than all other
> conversions I've had to do so far in the kernel, as you were using a
> "multiplexor" function for all of these files.
> 
> So, I've broken it up into individual show/store sysfs functions, and
> cleaned up a bunch of debug structures that aren't needed and shouldn't
> be exported to the rest of the kernel.
> 
> I've also fixed up some void * usage in the hv core, in patch 07, to
> make things simpler and not so "magic" when dealing with these pages.
> If you could review that one closely to ensure I didn't mess anything
> up, I would appreciate it.
> 
> Also, are all of these files really needed for sysfs?  They seem to be
> all debugging stuff, shouldn't they go into debugfs if you really
> need/use them anymore?
> 
> KY, could you test these out?  I don't have access to a hv system at the
> moment.  I'll wait for your ack before applying them to any of my trees.
> 
> thanks,

Will do. Thank you.

K. Y
> 
> greg k-h

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/1] Drivers: input: serio: New driver to support Hyper-V synthetic keyboard

[KY Srinivasan]

> -Original Message-
> From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> Sent: Monday, September 16, 2013 1:21 AM
> To: KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; linux-in...@vger.kernel.org;
> dmitry.torok...@gmail.com; vojt...@suse.cz; o...@aepfle.de;
> a...@canonical.com; jasow...@redhat.com
> Subject: Re: [PATCH 1/1] Drivers: input: serio: New driver to support Hyper-V
> synthetic keyboard
> 
> The main thing is that could you improve the error handling in
> hv_kbd_on_channel_callback() explained inline.

Thanks Dan. I will address all the relevant issues you have pointed out.
I have answered/responded to your comments in-line.
> 
> On Sun, Sep 15, 2013 at 10:28:54PM -0700, K. Y. Srinivasan wrote:
> > Add a new driver to support synthetic keyboard. On the next generation
> > Hyper-V guest firmware, many legacy devices will not be emulated and this
> > driver will be required.
> >
> > I would like to thank Vojtech Pavlik  for helping me with 
> > the
> > details of the AT keyboard driver.
> >
> > Signed-off-by: K. Y. Srinivasan 
> > ---
> >  drivers/input/serio/Kconfig   |7 +
> >  drivers/input/serio/Makefile  |1 +
> >  drivers/input/serio/hyperv-keyboard.c |  379
> +
> >  3 files changed, 387 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/input/serio/hyperv-keyboard.c
> >
> > diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
> > index 1e691a3..f3996e7 100644
> > --- a/drivers/input/serio/Kconfig
> > +++ b/drivers/input/serio/Kconfig
> > @@ -267,4 +267,11 @@ config SERIO_OLPC_APSP
> >   To compile this driver as a module, choose M here: the module will
> >   be called olpc_apsp.
> >
> > +config HYPERV_KEYBOARD
> > +   tristate "Microsoft Synthetic Keyboard driver"
> > +   depends on HYPERV
> > +   default HYPERV
> > +   help
> > + Select this option to enable the Hyper-V Keyboard driver.
> > +
> >  endif
> > diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
> > index 12298b1..815d874 100644
> > --- a/drivers/input/serio/Makefile
> > +++ b/drivers/input/serio/Makefile
> > @@ -28,3 +28,4 @@ obj-$(CONFIG_SERIO_ALTERA_PS2)+= altera_ps2.o
> >  obj-$(CONFIG_SERIO_ARC_PS2)+= arc_ps2.o
> >  obj-$(CONFIG_SERIO_APBPS2) += apbps2.o
> >  obj-$(CONFIG_SERIO_OLPC_APSP)  += olpc_apsp.o
> > +obj-$(CONFIG_HYPERV_KEYBOARD)  += hyperv-keyboard.o
> > diff --git a/drivers/input/serio/hyperv-keyboard.c
> b/drivers/input/serio/hyperv-keyboard.c
> > new file mode 100644
> > index 000..0d4625f
> > --- /dev/null
> > +++ b/drivers/input/serio/hyperv-keyboard.c
> > @@ -0,0 +1,379 @@
> > +/*
> > + *  Copyright (c) 2013, Microsoft Corporation.
> > + *
> > + *  This program is free software; you can redistribute it and/or modify it
> > + *  under the terms and conditions of the GNU General Public License,
> > + *  version 2, as published by the Free Software Foundation.
> > + *
> > + *  This program is distributed in the hope it will be useful, but WITHOUT
> > + *  ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> or
> > + *  FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
> for
> > + *  more details.
> > + */
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +
> 
> Extra blank line.
> 
> > +/*
> > + * Current version 1.0
> > + *
> > + */
> > +#define SYNTH_KBD_VERSION_MAJOR 1
> > +#define SYNTH_KBD_VERSION_MINOR0
> > +#define SYNTH_KBD_VERSION  (SYNTH_KBD_VERSION_MINOR |
> \
> > +(SYNTH_KBD_VERSION_MAJOR << 16))
> > +
> > +
> > +/*
> > + * Message types in the synthetic input protocol
> > + */
> > +enum synth_kbd_msg_type {
> > +   SYNTH_KBD_PROTOCOL_REQUEST = 1,
> > +   SYNTH_KBD_PROTOCOL_RESPONSE = 2,
> > +   SYNTH_KBD_EVENT = 3,
> > +   SYNTH_KBD_LED_INDICATORS = 4,
> > +};
> > +
> > +/*
> > + * Basic message structures.
> > + */
> > +struct synth_kbd_msg_hdr {
> > +   enum synth_kbd_msg_type type;
> > +};
> 
> Enum type is wrong here because sizeof(enum) is up to the compiler to
> decide.
> 
> > +
> > +struct synth_kbd_msg {
> > +   struct synth_kbd_msg_hdr header;
> > +   c

RE: [PATCH 1/1] Drivers: input: serio: New driver to support Hyper-V synthetic keyboard

[KY Srinivasan]

> -Original Message-
> From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com]
> Sent: Monday, September 16, 2013 8:20 AM
> To: KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; linux-in...@vger.kernel.org; vojt...@suse.cz;
> o...@aepfle.de; a...@canonical.com; jasow...@redhat.com
> Subject: Re: [PATCH 1/1] Drivers: input: serio: New driver to support Hyper-V
> synthetic keyboard
> 
> Hi K. Y.
> 
> On Sun, Sep 15, 2013 at 10:28:54PM -0700, K. Y. Srinivasan wrote:
> > Add a new driver to support synthetic keyboard. On the next generation
> > Hyper-V guest firmware, many legacy devices will not be emulated and this
> > driver will be required.
> >
> > I would like to thank Vojtech Pavlik  for helping me with 
> > the
> > details of the AT keyboard driver.
> >
> 
> In addition to what Dan said:
> 
> > +
> > +struct synth_kbd_protocol_response {
> > +   struct synth_kbd_msg_hdr header;
> > +   u32 accepted:1;
> > +   u32 reserved:31;
> > +};
> 
> Use of bitfields for on the wire structures makes me uneasy. I know that
> currently you only going to run LE on LE, but still, maybe using
> explicit shifts and masks would be better,

This definition of the data structure is defined by the host. I will see what I
can do here.

> 
> > +
> > +struct synth_kbd_keystroke {
> > +   struct synth_kbd_msg_hdr header;
> > +   u16 make_code;
> > +   u16 reserved0;
> > +   u32 is_unicode:1;
> > +   u32 is_break:1;
> > +   u32 is_e0:1;
> > +   u32 is_e1:1;
> > +   u32 reserved:28;
> > +};
> 
> Same here.
> 
> > +
> > +
> > +#define HK_MAXIMUM_MESSAGE_SIZE 256
> > +
> > +#define KBD_VSC_SEND_RING_BUFFER_SIZE  (10 * PAGE_SIZE)
> > +#define KBD_VSC_RECV_RING_BUFFER_SIZE  (10 * PAGE_SIZE)
> > +
> > +#define XTKBD_EMUL0 0xe0
> > +#define XTKBD_EMUL1 0xe1
> > +#define XTKBD_RELEASE   0x80
> > +
> > +
> > +/*
> > + * Represents a keyboard device
> > + */
> > +struct hv_kbd_dev {
> > +   unsigned char keycode[256];
> 
> I do not see anything using keycode table? This is wrong level for it
> regardless.
> 
> > +   struct hv_device*device;
> > +   struct synth_kbd_protocol_request protocol_req;
> > +   struct synth_kbd_protocol_response protocol_resp;
> > +   /* Synchronize the request/response if needed */
> > +   struct completion   wait_event;
> > +   struct serio*hv_serio;
> > +};
> > +
> > +
> > +static struct hv_kbd_dev *hv_kbd_alloc_device(struct hv_device *device)
> > +{
> > +   struct hv_kbd_dev *kbd_dev;
> > +   struct serio*hv_serio;
> 
> You are aligning with tabs half of declarations, leaving the others. Can
> we not align at all?
> 
> > +
> > +   kbd_dev = kzalloc(sizeof(struct hv_kbd_dev), GFP_KERNEL);
> > +
> > +   if (!kbd_dev)
> > +   return NULL;
> > +
> > +   hv_serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
> > +
> > +   if (hv_serio == NULL) {
> > +   kfree(kbd_dev);
> > +   return NULL;
> > +   }
> > +
> > +   hv_serio->id.type   = SERIO_8042_XL;
> > +   strlcpy(hv_serio->name, dev_name(&device->device),
> > +   sizeof(hv_serio->name));
> > +   strlcpy(hv_serio->phys, dev_name(&device->device),
> > +   sizeof(hv_serio->phys));
> > +   hv_serio->dev.parent  = &device->device;
> 
> Do you also want to set up serio's parent device to point to hv device?

Is there an issue here; I could setup the parent as the vmbus device.

> 
> > +
> > +
> > +   kbd_dev->device = device;
> > +   kbd_dev->hv_serio = hv_serio;
> > +   hv_set_drvdata(device, kbd_dev);
> > +   init_completion(&kbd_dev->wait_event);
> > +
> > +   return kbd_dev;
> > +}
> > +
> > +static void hv_kbd_free_device(struct hv_kbd_dev *device)
> > +{
> > +   serio_unregister_port(device->hv_serio);
> > +   kfree(device->hv_serio);
> 
> Serio ports are refcounted, do not free them explicitly after they have
> been registered.

Thank you. I will fix this.

> 
> > +   hv_set_drvdata(device->device, NULL);
> > +   kfree(device);
> > +}
> 
> 
> Thanks.


Thank you!.

Regards,

K. Y
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/1] Drivers: input: serio: New driver to support Hyper-V synthetic keyboard

[KY Srinivasan]

> -Original Message-
> From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> Sent: Monday, September 16, 2013 8:06 AM
> To: KY Srinivasan
> Cc: o...@aepfle.de; gre...@linuxfoundation.org; jasow...@redhat.com;
> dmitry.torok...@gmail.com; linux-kernel@vger.kernel.org; vojt...@suse.cz;
> linux-in...@vger.kernel.org; a...@canonical.com; de...@linuxdriverproject.org
> Subject: Re: [PATCH 1/1] Drivers: input: serio: New driver to support Hyper-V
> synthetic keyboard
> 
> On Mon, Sep 16, 2013 at 02:46:24PM +, KY Srinivasan wrote:
> > > > +   case VM_PKT_DATA_INBAND:
> > > > +   hv_kbd_on_receive(device, desc);
> > >
> > > This is the error handling I mentioned at the top.  hv_kbd_on_receive()
> > > doesn't take into consideration the amount of data we recieved, it
> > > trusts the offset we recieved from the user.  There is an out of bounds
> > > read.
> >
> > What user are you referring to. The message is sent by the host - the user
> keystroke
> > is normalized into a fixed size packet by the host and sent to the  guest. 
> > We will
> parse this
> > packet, based on the host specified layout here.
> >
> 
> The user means the hypervisor, yes.
> 
> I don't want the hypervisor accessing outside of the buffer.  It is
> robustness issue.  Just check the offset against "bytes_recvd".  It's
> not complicated.

At the outset, let me apologize for not understanding your concern.
You say: " I don't want the hypervisor accessing outside of the buffer"
Where did you see the hypervisor accessing anything outside the buffer?
The buffer is allocated by this driver and a packet from vmbus is read into this
buffer - this is the call to vmbus_recvpacket(). If the specified buffer is 
smaller
than the packet that needs to be read, then nothing will be read. Once the read
completes, we can be sure we have read a valid packet and can proceed to parse 
it in
this driver.

I don't want to be argumentative, I am just not seeing the issue.

Regards,

K. Y
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/1] Drivers: input: serio: New driver to support Hyper-V synthetic keyboard

[KY Srinivasan]

> -Original Message-
> From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com]
> Sent: Monday, September 16, 2013 10:10 AM
> To: KY Srinivasan
> Cc: Dan Carpenter; o...@aepfle.de; gre...@linuxfoundation.org;
> jasow...@redhat.com; linux-kernel@vger.kernel.org; vojt...@suse.cz; linux-
> in...@vger.kernel.org; a...@canonical.com; de...@linuxdriverproject.org
> Subject: Re: [PATCH 1/1] Drivers: input: serio: New driver to support Hyper-V
> synthetic keyboard
> 
> On Mon, Sep 16, 2013 at 04:56:03PM +, KY Srinivasan wrote:
> >
> >
> > > -Original Message-
> > > From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> > > Sent: Monday, September 16, 2013 8:06 AM
> > > To: KY Srinivasan
> > > Cc: o...@aepfle.de; gre...@linuxfoundation.org; jasow...@redhat.com;
> > > dmitry.torok...@gmail.com; linux-kernel@vger.kernel.org;
> vojt...@suse.cz;
> > > linux-in...@vger.kernel.org; a...@canonical.com;
> de...@linuxdriverproject.org
> > > Subject: Re: [PATCH 1/1] Drivers: input: serio: New driver to support 
> > > Hyper-V
> > > synthetic keyboard
> > >
> > > On Mon, Sep 16, 2013 at 02:46:24PM +, KY Srinivasan wrote:
> > > > > > +   case VM_PKT_DATA_INBAND:
> > > > > > +   hv_kbd_on_receive(device, desc);
> > > > >
> > > > > This is the error handling I mentioned at the top.  
> > > > > hv_kbd_on_receive()
> > > > > doesn't take into consideration the amount of data we recieved, it
> > > > > trusts the offset we recieved from the user.  There is an out of 
> > > > > bounds
> > > > > read.
> > > >
> > > > What user are you referring to. The message is sent by the host - the 
> > > > user
> > > keystroke
> > > > is normalized into a fixed size packet by the host and sent to the  
> > > > guest. We
> will
> > > parse this
> > > > packet, based on the host specified layout here.
> > > >
> > >
> > > The user means the hypervisor, yes.
> > >
> > > I don't want the hypervisor accessing outside of the buffer.  It is
> > > robustness issue.  Just check the offset against "bytes_recvd".  It's
> > > not complicated.
> >
> > At the outset, let me apologize for not understanding your concern.
> > You say: " I don't want the hypervisor accessing outside of the buffer"
> > Where did you see the hypervisor accessing anything outside the buffer?
> > The buffer is allocated by this driver and a packet from vmbus is read into 
> > this
> > buffer - this is the call to vmbus_recvpacket(). If the specified buffer is 
> > smaller
> > than the packet that needs to be read, then nothing will be read. Once the 
> > read
> > completes, we can be sure we have read a valid packet and can proceed to
> parse it in
> > this driver.
> 
> The concern is that number of bytes received and contents of a packet
> are not in sync. Imagine if we were told that 16 butes was received but
> in the packet offset is 78. Then we'll try reading well past the buffer
> boundary that we allocated for the packets.

I am not sure how this would be the case. Following are the semantics of the 
function
vmbus_recvpacket_raw():

If the packet to be read is larger than the buffer specified; nothing will be 
read and 
appropriate error is returned. If a  packet is read, the complete packet is 
read and 
so we can safely peek into this packet based on the information in the header.

Regards,


K. Y
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/1] Drivers: input: serio: New driver to support Hyper-V synthetic keyboard

[KY Srinivasan]

> -Original Message-
> From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> Sent: Monday, September 16, 2013 11:33 AM
> To: KY Srinivasan
> Cc: Dmitry Torokhov; o...@aepfle.de; gre...@linuxfoundation.org;
> jasow...@redhat.com; linux-kernel@vger.kernel.org; vojt...@suse.cz; linux-
> in...@vger.kernel.org; a...@canonical.com; de...@linuxdriverproject.org
> Subject: Re: [PATCH 1/1] Drivers: input: serio: New driver to support Hyper-V
> synthetic keyboard
> 
> Just roll something like the following into your patch.
> 
> regards,
> dan carpenter
> 
> diff --git a/drivers/input/serio/hyperv-keyboard.c 
> b/drivers/input/serio/hyperv-
> keyboard.c
> index 0d4625f..262721b 100644
> --- a/drivers/input/serio/hyperv-keyboard.c
> +++ b/drivers/input/serio/hyperv-keyboard.c
> @@ -151,15 +151,18 @@ static void hv_kbd_free_device(struct hv_kbd_dev
> *device)
>  }
> 
>  static void hv_kbd_on_receive(struct hv_device *device,
> - struct vmpacket_descriptor *packet)
> +   struct vmpacket_descriptor *packet, size_t size)
>  {
>   struct synth_kbd_msg *msg;
>   struct hv_kbd_dev *kbd_dev = hv_get_drvdata(device);
>   struct synth_kbd_keystroke *ks_msg;
> + int offset;
>   u16 scan_code;
> 
> - msg = (struct synth_kbd_msg *)((unsigned long)packet +
> - (packet->offset8 << 3));
> + offset = packet->offset8 << 3;
> + if (offset + sizeof(struct synth_kbd_protocol_response) > size)
> + return;
> + msg = (void *)packet + offset;
> 
>   switch (msg->header.type) {
>   case SYNTH_KBD_PROTOCOL_RESPONSE:
> @@ -220,7 +223,7 @@ static void hv_kbd_on_channel_callback(void *context)
>   break;
> 
>   case VM_PKT_DATA_INBAND:
> - hv_kbd_on_receive(device, desc);
> + hv_kbd_on_receive(device, desc, bytes_recvd);
>   break;
> 
>   default:

Dan,

Rolling the changes you have indicated is not the issue; this can trivially be 
done.
My contention is that it is not needed given that the underlying function is 
already
doing that. Look at the function  vmbus_recvpacket_raw() in 
drivers/hv/channel.c.

Regards,

K. Y


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/1] Drivers: input: serio: New driver to support Hyper-V synthetic keyboard

[KY Srinivasan]

> -Original Message-
> From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> Sent: Monday, September 16, 2013 1:13 PM
> To: KY Srinivasan
> Cc: o...@aepfle.de; gre...@linuxfoundation.org; jasow...@redhat.com; Dmitry
> Torokhov; linux-kernel@vger.kernel.org; vojt...@suse.cz; linux-
> in...@vger.kernel.org; a...@canonical.com; de...@linuxdriverproject.org
> Subject: Re: [PATCH 1/1] Drivers: input: serio: New driver to support Hyper-V
> synthetic keyboard
> 
> On Mon, Sep 16, 2013 at 06:42:25PM +, KY Srinivasan wrote:
> > Dan,
> >
> > Rolling the changes you have indicated is not the issue; this can trivially 
> > be
> done.
> > My contention is that it is not needed given that the underlying function is
> already
> > doing that. Look at the function  vmbus_recvpacket_raw() in
> drivers/hv/channel.c.
> >
> 
> I'm confused.
> 
> There is no mention of ->offset8 in vmbus_recvpacket_raw().

As you can see the vmbus_recvpacket_raw() ensures that the complete
packet is read and if the buffer specified is not large enough nothing is 
read. The packet header has information about the length of the packet
and the offset where the payload is.  
> 
> It's a good idea to add a check there but the lower levels don't know
> about the sizeof(synth_kbd_protocol_response) so we would still need
> something like my check.

Why would the lower level code need to know  anything about the layout of a
particular message type. The lower level code is guaranteeing that a complete
packet has been read in and that should be sufficient - assuming we trust the 
host.

We have already spent more time on this than we should; I will make the 
necessary
changes.

Regards,

K. Y
> 
> regards,
> dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 00/13] hv: clean up dev_attr usage

[KY Srinivasan]

> -Original Message-
> From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org]
> Sent: Friday, September 13, 2013 11:33 AM
> To: KY Srinivasan; Haiyang Zhang
> Cc: de...@linuxdriverproject.org; linux-kernel@vger.kernel.org
> Subject: [PATCH 00/13] hv: clean up dev_attr usage
> 
> Hi,
> 
> Here's a set of 13 patches to get rid of the dev_attrs use in the hv bus
> code, as it will be going away soon.  It's _way_ bigger than all other
> conversions I've had to do so far in the kernel, as you were using a
> "multiplexor" function for all of these files.
> 
> So, I've broken it up into individual show/store sysfs functions, and
> cleaned up a bunch of debug structures that aren't needed and shouldn't
> be exported to the rest of the kernel.
> 
> I've also fixed up some void * usage in the hv core, in patch 07, to
> make things simpler and not so "magic" when dealing with these pages.
> If you could review that one closely to ensure I didn't mess anything
> up, I would appreciate it.
> 
> Also, are all of these files really needed for sysfs?  They seem to be
> all debugging stuff, shouldn't they go into debugfs if you really
> need/use them anymore?
> 
> KY, could you test these out?  I don't have access to a hv system at the
> moment.  I'll wait for your ack before applying them to any of my trees.

The patches look good and I tested them. The guest comes up and is functional.
I did notice though that the pending state appears to be a signed entity now 
which was not the
the case  before - I see a negative sign when I cat the client/server pending 
state.

Regards,

K. Y 
> 
> thanks,
> 
> greg k-h

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH V2 1/1] X86: Hyper-V: Get the local APIC timer frequency from the hypervisor

[KY Srinivasan]

> -Original Message-
> From: Gleb Natapov [mailto:g...@redhat.com]
> Sent: Tuesday, September 17, 2013 6:59 AM
> To: KY Srinivasan
> Cc: H. Peter Anvin; x...@kernel.org; gre...@linuxfoundation.org; linux-
> ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de;
> a...@canonical.com; jasow...@redhat.com; t...@linutronix.de;
> jbeul...@suse.com; b...@alien8.de
> Subject: Re: [PATCH V2 1/1] X86: Hyper-V: Get the local APIC timer frequency
> from the hypervisor
> 
> On Fri, Sep 13, 2013 at 02:28:35PM +, KY Srinivasan wrote:
> >
> >
> > > -Original Message-
> > > From: Gleb Natapov [mailto:g...@redhat.com]
> > > Sent: Friday, September 13, 2013 2:55 AM
> > > To: KY Srinivasan
> > > Cc: H. Peter Anvin; x...@kernel.org; gre...@linuxfoundation.org; linux-
> > > ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de;
> > > a...@canonical.com; jasow...@redhat.com; t...@linutronix.de;
> > > jbeul...@suse.com; b...@alien8.de
> > > Subject: Re: [PATCH V2 1/1] X86: Hyper-V: Get the local APIC timer 
> > > frequency
> > > from the hypervisor
> > >
> > > On Fri, Sep 13, 2013 at 01:43:09AM +, KY Srinivasan wrote:
> > > >
> > > >
> > > > > -Original Message-
> > > > > From: H. Peter Anvin [mailto:h...@zytor.com]
> > > > > Sent: Thursday, September 12, 2013 5:28 PM
> > > > > To: KY Srinivasan
> > > > > Cc: x...@kernel.org; gre...@linuxfoundation.org; linux-
> > > ker...@vger.kernel.org;
> > > > > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> > > > > jasow...@redhat.com; t...@linutronix.de; jbeul...@suse.com;
> > > b...@alien8.de
> > > > > Subject: Re: [PATCH V2 1/1] X86: Hyper-V: Get the local APIC timer
> frequency
> > > > > from the hypervisor
> > > > >
> > > > > On 09/12/2013 05:06 PM, KY Srinivasan wrote:
> > > > > >
> > > > > > Peter,
> > > > > >
> > > > > > Let me know if you want me to address any additional issues in this
> patch.
> > > > > >
> > > > >
> > > > > Please address Jan and Gleb's feedback.
> > > >
> > > > Gleb's feedback was a question and I answered that as I did Jan's 
> > > > feedback
> as
> > > well.
> > > > Gleb, Jan, please let me know if there is something else you want
> addressed
> > > here.
> > > >
> > > No, I am just interesting to know some details about the interface since
> > > I cannot find it documented in Hyper-V spec.
> >
> > Thanks Gleb. Here is the link for the latest Hyper-V specification:
> > http://www.microsoft.com/en-us/download/details.aspx?id=39289
> >
> > This spec talks about how migration is handled with regards to TSC.
> >
> All I can see is "15.4.3 Partition Reference TSC Mechanism". I see
> nothing about HV_X64_MSR_TSC_FREQUENCY specifically. To access this MSR
> a partition needs AccessFrequencyMsrs privilege, may be partition that
> cab be migrated does not have this privilege?

If you use the MSR to read the TSC frequency, then indeed when you migrate then 
there
is no guarantee that the TSC frequency won't change when you migrate. The 
approach
described in section 15.4.3 is the preferred approach that can accommodate 
migration.

Regards,

K. Y 
> 
> --
>   Gleb.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH V2 1/1] Drivers: input: serio: New driver to support Hyper-V synthetic keyboard

[KY Srinivasan]

> -Original Message-
> From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com]
> Sent: Wednesday, September 18, 2013 2:01 PM
> To: KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com; dan.carpen...@oracle.com; linux-
> in...@vger.kernel.org; vojt...@suse.cz
> Subject: Re: [PATCH V2 1/1] Drivers: input: serio: New driver to support 
> Hyper-V
> synthetic keyboard
> 
> Hi K.Y.,
> 
> On Tue, Sep 17, 2013 at 04:26:58PM -0700, K. Y. Srinivasan wrote:
> > Add a new driver to support synthetic keyboard. On the next generation
> > Hyper-V guest firmware, many legacy devices will not be emulated and this
> > driver will be required.
> >
> > I would like to thank Vojtech Pavlik  for helping me with 
> > the
> > details of the AT keyboard driver. I would also like to thank
> > Dan Carpenter  and
> > Dmitry Torokhov  for their detailed review of
> this
> > driver.
> >
> > I have addressed all the comments of Dan and Dmitry in this version of
> > the patch
> 
> This looks much better. Could you tell me if the patch below (on top of
> yours) still works?
> 
> Thanks.

Thank you. The code looks much better now. You forgot to initialize the 
port_data and
after I fixed that everything seems to work as it did before: Here is the patch 
I used:

> -Original Message-
> From: K. Y. Srinivasan [mailto:k...@microsoft.com]
> Sent: Wednesday, September 18, 2013 4:50 PM
> To: KY Srinivasan
> Subject: [PATCH 1/1] Drivers: input: serio: hyper-V: Initialize the port data
> correctly
> 
> 
> Signed-off-by: K. Y. Srinivasan 
> ---
>  drivers/input/serio/hyperv-keyboard.c |1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/input/serio/hyperv-keyboard.c 
> b/drivers/input/serio/hyperv-
> keyboard.c
> index 401fbdd..aff4152 100644
> --- a/drivers/input/serio/hyperv-keyboard.c
> +++ b/drivers/input/serio/hyperv-keyboard.c
> @@ -351,6 +351,7 @@ static int hv_kbd_probe(struct hv_device *hv_dev,
> 
>   hv_serio->dev.parent  = &hv_dev->device;
>   hv_serio->id.type = SERIO_8042_XL;
> + hv_serio->port_data = kbd_dev;
>   strlcpy(hv_serio->name, dev_name(&hv_dev->device),
>   sizeof(hv_serio->name));
>   strlcpy(hv_serio->phys, dev_name(&hv_dev->device),
> --
> 1.7.4.1


Once again; thank you for all your help.

Regards,

K. Y

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH V2 1/1] Drivers: input: serio: New driver to support Hyper-V synthetic keyboard

[KY Srinivasan]

> -Original Message-
> From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com]
> Sent: Thursday, September 19, 2013 8:53 AM
> To: KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com; dan.carpen...@oracle.com; linux-
> in...@vger.kernel.org; vojt...@suse.cz
> Subject: Re: [PATCH V2 1/1] Drivers: input: serio: New driver to support 
> Hyper-V
> synthetic keyboard
> 
> On Wed, Sep 18, 2013 at 11:27:51PM +, KY Srinivasan wrote:
> >
> >
> > > -Original Message-
> > > From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com]
> > > Sent: Wednesday, September 18, 2013 2:01 PM
> > > To: KY Srinivasan
> > > Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> > > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> > > jasow...@redhat.com; dan.carpen...@oracle.com; linux-
> > > in...@vger.kernel.org; vojt...@suse.cz
> > > Subject: Re: [PATCH V2 1/1] Drivers: input: serio: New driver to support
> Hyper-V
> > > synthetic keyboard
> > >
> > > Hi K.Y.,
> > >
> > > On Tue, Sep 17, 2013 at 04:26:58PM -0700, K. Y. Srinivasan wrote:
> > > > Add a new driver to support synthetic keyboard. On the next generation
> > > > Hyper-V guest firmware, many legacy devices will not be emulated and 
> > > > this
> > > > driver will be required.
> > > >
> > > > I would like to thank Vojtech Pavlik  for helping me 
> > > > with
> the
> > > > details of the AT keyboard driver. I would also like to thank
> > > > Dan Carpenter  and
> > > > Dmitry Torokhov  for their detailed review of
> > > this
> > > > driver.
> > > >
> > > > I have addressed all the comments of Dan and Dmitry in this version of
> > > > the patch
> > >
> > > This looks much better. Could you tell me if the patch below (on top of
> > > yours) still works?
> > >
> > > Thanks.
> >
> > Thank you. The code looks much better now. You forgot to initialize the
> port_data and
> > after I fixed that everything seems to work as it did before: Here is the 
> > patch I
> used:
> 
> Excellent, thank you for trying it out. I folded it all together and
> queued for 3.13.

Thank you.

Regards,

K. Y

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH V2 1/1] X86: Hyper-V: Get the local APIC timer frequency from the hypervisor

[KY Srinivasan]

> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: Wednesday, September 04, 2013 12:17 AM
> To: KY Srinivasan
> Cc: o...@aepfle.de; b...@alien8.de; a...@canonical.com; x...@kernel.org;
> t...@linutronix.de; de...@linuxdriverproject.org; gre...@linuxfoundation.org;
> jasow...@redhat.com; linux-kernel@vger.kernel.org; h...@zytor.com
> Subject: Re: [PATCH V2 1/1] X86: Hyper-V: Get the local APIC timer frequency
> from the hypervisor
> 
> >>> On 03.09.13 at 20:30, "K. Y. Srinivasan"  wrote:
> > @@ -76,6 +80,26 @@ static void __init ms_hyperv_init_platform(void)
> > printk(KERN_INFO "HyperV: features 0x%x, hints 0x%x\n",
> >ms_hyperv.features, ms_hyperv.hints);
> >
> > +   if (ms_hyperv.features & HV_X64_MSR_APIC_FREQUENCY_AVAILABLE) {
> > +   /*
> > +* Get the APIC frequency.
> > +*/
> > +   rdmsrl(HV_X64_MSR_APIC_FREQUENCY, hv_lapic_frequency);
> > +   hv_lapic_frequency /= HZ;
> > +   lapic_timer_frequency = hv_lapic_frequency;
> > +   printk(KERN_INFO "HyperV: LAPIC Timer Frequency: %#x\n",
> > +   lapic_timer_frequency);
> > +
> > +   /*
> > +* On Hyper-V, when we are booting off an EFI firmware stack,
> > +* we do not have many legacy devices including PIC, PIT etc.
> > +*/
> > +   if (efi_enabled(EFI_BOOT)) {
> > +   printk(KERN_INFO "HyperV: Using null_legacy_pic\n");
> > +   legacy_pic = &null_legacy_pic;
> > +   }
> 
> And this check is really connected to the feature check around the
> whole block, rather than being independent? (I'd also think that
> this latter message would suffice to be KERN_DEBUG).

I felt it was safer to first check for the feature  since if the feature is not
there, we need to calibrate based on PIT. Furthermore, the feature is available
even in legacy environments when we are not booting on an EFI stack.

Regards,

K. Y

> 
> Jan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH V2 1/1] X86: Hyper-V: Get the local APIC timer frequency from the hypervisor

[KY Srinivasan]

> -Original Message-
> From: Gleb Natapov [mailto:g...@redhat.com]
> Sent: Wednesday, September 04, 2013 2:40 AM
> To: KY Srinivasan
> Cc: x...@kernel.org; gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com; t...@linutronix.de; h...@zytor.com;
> jbeul...@suse.com; b...@alien8.de
> Subject: Re: [PATCH V2 1/1] X86: Hyper-V: Get the local APIC timer frequency
> from the hypervisor
> 
> On Tue, Sep 03, 2013 at 11:30:23AM -0700, K. Y. Srinivasan wrote:
> > Hyper-V supports a mechanism for retrieving the local APIC frequency.Use 
> > this
> and bypass
> > the calibration code in the kernel. This would allow us to boot the Linux 
> > kernel
> as a
> > "modern VM" on Hyper-V where many of the legacy devices (such as PIT) are
> not emulated.
> >
> > I would like to thank Olaf Hering , Jan Beulich
>  and
> > H. Peter Anvin  for their help in this effort.
> >
> > In this version of the patch, I have addressed Jan's comments.
> >
> > Signed-off-by: K. Y. Srinivasan 
> > ---
> >  arch/x86/include/uapi/asm/hyperv.h |   19 +++
> >  arch/x86/kernel/cpu/mshyperv.c |   24 
> >  2 files changed, 43 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/x86/include/uapi/asm/hyperv.h
> b/arch/x86/include/uapi/asm/hyperv.h
> > index b80420b..b8f1c01 100644
> > --- a/arch/x86/include/uapi/asm/hyperv.h
> > +++ b/arch/x86/include/uapi/asm/hyperv.h
> > @@ -27,6 +27,19 @@
> >  #define HV_X64_MSR_VP_RUNTIME_AVAILABLE(1 << 0)
> >  /* Partition Reference Counter (HV_X64_MSR_TIME_REF_COUNT) available*/
> >  #define HV_X64_MSR_TIME_REF_COUNT_AVAILABLE(1 << 1)
> > +
> > +/*
> > + * There is a single feature flag that signifies the presence of the MSR
> > + * that can be used to retrieve both the local APIC Timer frequency as
> > + * well as the TSC frequency.
> > + */
> > +
> > +/* Local APIC timer frequency MSR (HV_X64_MSR_APIC_FREQUENCY) is
> available */
> > +#define HV_X64_MSR_APIC_FREQUENCY_AVAILABLE (1 << 11)
> > +
> > +/* TSC frequency MSR (HV_X64_MSR_TSC_FREQUENCY) is available */
> > +#define HV_X64_MSR_TSC_FREQUENCY_AVAILABLE (1 << 11)
> > +
> >  /*
> >   * Basic SynIC MSRs (HV_X64_MSR_SCONTROL through HV_X64_MSR_EOM
> >   * and HV_X64_MSR_SINT0 through HV_X64_MSR_SINT15) available
> > @@ -136,6 +149,12 @@
> >  /* MSR used to read the per-partition time reference counter */
> >  #define HV_X64_MSR_TIME_REF_COUNT  0x4020
> >
> > +/* MSR used to retrieve the TSC frequency */
> > +#define HV_X64_MSR_TSC_FREQUENCY   0x4022
> > +
> You do not use this MSR in the patch, but in general how it suppose to
> work during migration if host TSC frequency changes?

TSC related migration issues are distinct from how we calibrate the TSC 
frequency. This MSR
allows you to retrieve the TSC frequency without having to do explicit 
calibration.

To address the migration issues; the hypervisor provides additional information 
on the host
that you are running that can be used to compensate for differences in TSC 
across hosts.  

Regards,

K. Y
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/1] Drivers: hv: util: Fix a bug in version negotiation code for util services

[KY Srinivasan]

> -Original Message-
> From: Olaf Hering [mailto:o...@aepfle.de]
> Sent: Wednesday, September 04, 2013 8:07 AM
> To: KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/1] Drivers: hv: util: Fix a bug in version negotiation 
> code for
> util services
> 
> On Wed, Sep 04, Olaf Hering wrote:
> 
> > I suggest to let callers deal with error handling. Also as a cleanup,
> > vmbus_prep_negotiate_resp does not make use of the negop passed to it.
> > So that arg can be removed.
> 
> Simplify vmbus_prep_negotiate_resp. If we take the optimistic approach
> that negotiation will not fail for any of the callers of
> vmbus_prep_negotiate_resp this patch on top of yours should fix 2008 and
> 2012r2.
Thanks Olaf. I would prefer that we explicitly fail the negotiations than 
assume that it will succeed.
Essentially, I want the same behavior as what we had before but only for the 
final version being
negotiated. If it is ok with you, I can spin up the patch and send it out.

Regards,

K. Y

> 
> Olaf
> 
> ---
>  drivers/hv/channel_mgmt.c | 22 +-
>  drivers/hv/hv_kvp.c   |  8 
>  drivers/hv/hv_snapshot.c  |  3 +--
>  drivers/hv/hv_util.c  |  7 +++
>  include/linux/hyperv.h|  4 +---
>  5 files changed, 18 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index 12ec8c8..dcaad3e 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -41,7 +41,6 @@ struct vmbus_channel_message_table_entry {
>  /**
>   * vmbus_prep_negotiate_resp() - Create default response for Hyper-V
> Negotiate message
>   * @icmsghdrp: Pointer to msg header structure
> - * @icmsg_negotiate: Pointer to negotiate message structure
>   * @buf: Raw buffer channel data
>   *
>   * @icmsghdrp is of type &struct icmsg_hdr.
> @@ -54,10 +53,10 @@ struct vmbus_channel_message_table_entry {
>   *
>   * Mainly used by Hyper-V drivers.
>   */
> -bool vmbus_prep_negotiate_resp(struct icmsg_hdr *icmsghdrp,
> - struct icmsg_negotiate *negop, u8 *buf,
> +bool vmbus_prep_negotiate_resp(struct icmsg_hdr *icmsghdrp, u8 *buf,
>   int fw_version, int srv_version)
>  {
> + struct icmsg_negotiate *negop;
>   int icframe_major, icframe_minor;
>   int icmsg_major, icmsg_minor;
>   int fw_major, fw_minor;
> @@ -65,7 +64,6 @@ bool vmbus_prep_negotiate_resp(struct icmsg_hdr
> *icmsghdrp,
>   int i;
>   bool found_match = false;
> 
> - icmsghdrp->icmsgsize = 0x10;
>   fw_major = (fw_version >> 16);
>   fw_minor = (fw_version & 0x);
> 
> @@ -116,19 +114,17 @@ bool vmbus_prep_negotiate_resp(struct icmsg_hdr
> *icmsghdrp,
>* version numbers we can support.
>*/
> 
> -fw_error:
> - if (!found_match) {
> - negop->icframe_vercnt = 0;
> - negop->icmsg_vercnt = 0;
> - } else {
> + if (found_match) {
> + icmsghdrp->icmsgsize = 0x10;
>   negop->icframe_vercnt = 1;
>   negop->icmsg_vercnt = 1;
> + negop->icversion_data[0].major = icframe_major;
> + negop->icversion_data[0].minor = icframe_minor;
> + negop->icversion_data[1].major = icmsg_major;
> + negop->icversion_data[1].minor = icmsg_minor;
>   }
> 
> - negop->icversion_data[0].major = icframe_major;
> - negop->icversion_data[0].minor = icframe_minor;
> - negop->icversion_data[1].major = icmsg_major;
> - negop->icversion_data[1].minor = icmsg_minor;
> +fw_error:
>   return found_match;
>  }
> 
> diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
> index 5312720..31e338a 100644
> --- a/drivers/hv/hv_kvp.c
> +++ b/drivers/hv/hv_kvp.c
> @@ -584,7 +584,6 @@ void hv_kvp_onchannelcallback(void *context)
>   struct hv_kvp_msg *kvp_msg;
> 
>   struct icmsg_hdr *icmsghdrp;
> - struct icmsg_negotiate *negop = NULL;
> 
>   if (kvp_transaction.active) {
>   /*
> @@ -607,14 +606,15 @@ void hv_kvp_onchannelcallback(void *context)
>* We start with win8 version and if the host cannot
>* support that we use the previous version.
>*/
> - if (vmbus_prep_negotiate_resp(icmsghdrp, negop,
> + if (vmbus_prep_negotiate_resp(icmsghdrp,
>recv_buffer, UTIL_FW_MAJOR_MINOR,
>WIN8_SRV_MAJOR_MINOR))
>  

RE: [PATCH] Drivers: hv: vmbus: fix error return code in vmbus_connect()

[KY Srinivasan]

> -Original Message-
> From: Wei Yongjun [mailto:weiyj...@gmail.com]
> Sent: Wednesday, September 11, 2013 4:20 AM
> To: KY Srinivasan; Haiyang Zhang
> Cc: yongjun_...@trendmicro.com.cn; de...@linuxdriverproject.org; linux-
> ker...@vger.kernel.org
> Subject: [PATCH] Drivers: hv: vmbus: fix error return code in vmbus_connect()
> 
> From: Wei Yongjun 
> 
> Fix to return -EINVAL in the version check error handling
> case instead of 0, as done elsewhere in this function.

The return will not be zero in this case. If you look at the function 
vmbus_negotiate_version(), in case the host refuses the version, the
return value will be set to -ECONNREFUSED

Regards,

K. Y
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/