RE: [EXTERNAL] [PATCH 1/1] scsi: Fix possible buffer overflows in storvsc_queuecommand
> -Original Message- > From: Xiaohui Zhang > Sent: Tuesday, December 8, 2020 5:19 AM > To: Xiaohui Zhang ; KY Srinivasan > ; Haiyang Zhang ; Stephen > Hemminger ; Wei Liu ; > James E.J. Bottomley ; Martin K. Petersen > ; linux-hyp...@vger.kernel.org; linux- > s...@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: [EXTERNAL] [PATCH 1/1] scsi: Fix possible buffer overflows in > storvsc_queuecommand > > From: Zhang Xiaohui > > storvsc_queuecommand() calls memcpy() without checking the destination size > may trigger a buffer overflower, which a local user could use to cause denial > of > service or the execution of arbitrary code. > Fix it by putting the length check before calling memcpy(). > > Signed-off-by: Zhang Xiaohui > --- > drivers/scsi/storvsc_drv.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index > 0c65fbd41..09b60a4c0 100644 > --- a/drivers/scsi/storvsc_drv.c > +++ b/drivers/scsi/storvsc_drv.c > @@ -1729,6 +1729,8 @@ static int storvsc_queuecommand(struct Scsi_Host > *host, struct scsi_cmnd *scmnd) > > vm_srb->cdb_length = scmnd->cmd_len; > > + if (vm_srb->cdb_length > STORVSC_MAX_CMD_LEN) > + vm_srb->cdb_length = STORVSC_MAX_CMD_LEN; > memcpy(vm_srb->cdb, scmnd->cmnd, vm_srb->cdb_length); The data structure is sized correctly to handle the max command length. Besides your check is bogus - you cannot truncate the command! K. Y > > sgl = (struct scatterlist *)scsi_sglist(scmnd); > -- > 2.17.1
RE: exfat filesystem
> -Original Message- > From: Valdis Kletnieks On Behalf Of Valdis Kletnieks > Sent: Tuesday, July 9, 2019 9:51 AM > To: KY Srinivasan > Cc: Matthew Wilcox ; Theodore Ts'o > ; Alexander Viro ; Greg Kroah- > Hartman ; linux-fsde...@vger.kernel.org; > linux-kernel@vger.kernel.org; de...@driverdev.osuosl.org; Sasha Levin > > Subject: Re: exfat filesystem > > On Tue, 09 Jul 2019 16:39:31 -, KY Srinivasan said: > > > Let me dig up the details here. > > In case this helps clarify the chain of events, the code in question is the > Samsung code mentioned here, updated to 5.2 kernel > > "We know that Microsoft has done patent troll shakedowns in the past on > Linux products related to the exfat filesystem. While we at Conservancy > were successful in getting the code that implements exfat for Linux released > under GPL (by Samsung), that code has not been upstreamed into Linux. So, > Microsoft has not included any patents they might hold on exfat into the > patent non-aggression pact." > > https://sfconservancy.org/blog/2018/oct/10/microsoft-oin-exfat/ > > (Link in that para points here): > https://sfconservancy.org/news/2013/aug/16/exfat-samsung/ > Thanks Valdis. I have started an internal thread on this; will get back ASAP. K. Y
RE: exfat filesystem
-Original Message- From: Matthew Wilcox Sent: Tuesday, July 9, 2019 8:49 AM To: Theodore Ts'o ; Valdis Klētnieks ; Alexander Viro ; Greg Kroah-Hartman ; linux-fsde...@vger.kernel.org; linux-kernel@vger.kernel.org; de...@driverdev.osuosl.org; KY Srinivasan Cc: Sasha Levin Subject: exfat filesystem On Tue, Jul 09, 2019 at 11:30:39AM -0400, Theodore Ts'o wrote: > On Tue, Jul 09, 2019 at 04:21:36AM -0700, Matthew Wilcox wrote: > > How does > > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fww > > w.zdnet.com%2Farticle%2Fmicrosoft-open-sources-its-entire-patent-por > > tfolio%2Fdata=02%7C01%7Ckys%40microsoft.com%7Cd73183ff28c94bbbf > > 6dd08d70484f009%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6369828 > > 41322780798sdata=TCSgqe0h4FYaA5BBGVJl98WFBqbEHSo8B0FhlfTYVVA%3D > > reserved=0 > > change your personal opinion? > > According to SFC's legal analysis, Microsoft joining the OIN doesn't > mean that the eXFAT patents are covered, unless *Microsoft* > contributes the code to the Linux usptream kernel. That's because the > OIN is governed by the Linux System Definition, and until MS > contributes code which covered by the exFAT patents, it doesn't count. > > For more details: > > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsfco > nservancy.org%2Fblog%2F2018%2Foct%2F10%2Fmicrosoft-oin-exfat%2Fda > ta=02%7C01%7Ckys%40microsoft.com%7Cd73183ff28c94bbbf6dd08d70484f009%7C > 72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636982841322780798sdat > a=y%2BhZFhjIXUrFVn5%2FN%2BRVxRQWzYs2QI5V1jM8SDPN2dg%3Dreserved=0 > > (This is not legal advice, and I am not a lawyer.) >Interesting analysis. It seems to me that the correct forms would be observed >if someone suitably senior at Microsoft accepted the work from >Valdis and >submitted it with their sign-off. KY, how about it? Matthew, Let me dig up the details here. K. Y
RE: [PATCH] scsi: storvsc: Add ability to change scsi queue depth
> -Original Message- > From: Michael Kelley > Sent: Saturday, June 15, 2019 11:19 AM > To: brandonbonaby94 ; KY Srinivasan > ; Haiyang Zhang ; Stephen > Hemminger ; sas...@kernel.org; > j...@linux.ibm.com; martin.peter...@oracle.com > Cc: brandonbonaby94 ; linux- > hyp...@vger.kernel.org; linux-s...@vger.kernel.org; linux- > ker...@vger.kernel.org > Subject: RE: [PATCH] scsi: storvsc: Add ability to change scsi queue depth > > From: Branden Bonaby Sent: Friday, June > 14, 2019 4:48 PM > > > > Adding functionality to allow the SCSI queue depth to be changed, > > by utilizing the "scsi_change_queue_depth" function. > > > > Signed-off-by: Branden Bonaby > > --- > > drivers/scsi/storvsc_drv.c | 11 +++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c > > index 8472de1007ff..719ca9906fc2 100644 > > --- a/drivers/scsi/storvsc_drv.c > > +++ b/drivers/scsi/storvsc_drv.c > > @@ -387,6 +387,7 @@ enum storvsc_request_type { > > > > static int storvsc_ringbuffer_size = (128 * 1024); > > static u32 max_outstanding_req_per_channel; > > +static int storvsc_change_queue_depth(struct scsi_device *sdev, int > queue_depth); > > > > static int storvsc_vcpus_per_sub_channel = 4; > > > > @@ -1711,6 +1712,7 @@ static struct scsi_host_template scsi_driver = { > > .dma_boundary = PAGE_SIZE-1, > > .no_write_same =1, > > .track_queue_depth =1, > > + .change_queue_depth = storvsc_change_queue_depth, > > }; > > > > enum { > > @@ -1917,6 +1919,15 @@ static int storvsc_probe(struct hv_device > *device, > > return ret; > > } > > > > +/* Change a scsi target's queue depth */ > > +static int storvsc_change_queue_depth(struct scsi_device *sdev, int > queue_depth) > > +{ > > + if (queue_depth > scsi_driver.can_queue){ > > + queue_depth = scsi_driver.can_queue; > > + } > > + return scsi_change_queue_depth(sdev, queue_depth); > > +} > > + > > static int storvsc_remove(struct hv_device *dev) > > { > > struct storvsc_device *stor_device = hv_get_drvdata(dev); > > -- > > 2.17.1 > > Reviewed-by: Michael Kelley Reviewed-by: K. Y. Srinivasan
RE: [PATCH] hyperv: a potential NULL pointer dereference
> -Original Message- > From: Thomas Gleixner > Sent: Wednesday, March 20, 2019 3:21 AM > To: KY Srinivasan > Cc: Kangjie Lu ; pakki...@umn.edu; Haiyang Zhang > ; Stephen Hemminger > ; Sasha Levin ; Ingo Molnar > ; Borislav Petkov ; H. Peter Anvin > ; x...@kernel.org; linux-hyp...@vger.kernel.org; linux- > ker...@vger.kernel.org > Subject: RE: [PATCH] hyperv: a potential NULL pointer dereference > > On Thu, 14 Mar 2019, KY Srinivasan wrote: > > > -Original Message- > > > From: Kangjie Lu > > > Sent: Wednesday, March 13, 2019 10:47 PM > > > To: k...@umn.edu > > > Cc: pakki...@umn.edu; KY Srinivasan ; Haiyang > Zhang > > > ; Stephen Hemminger > > > ; Sasha Levin ; Thomas > > > Gleixner ; Ingo Molnar ; > Borislav > > > Petkov ; H. Peter Anvin ; > x...@kernel.org; > > > linux-hyp...@vger.kernel.org; linux-kernel@vger.kernel.org > > > Subject: [PATCH] hyperv: a potential NULL pointer dereference > > > > > > In case alloc_page, the fix returns -ENOMEM to avoid the potential > > > NULL pointer dereference. > > > > > Thanks. > > > > > Signed-off-by: Kangjie Lu > > Signed-off-by: K. Y. Srinivasan > > Did you mean: Reviewed-by or Acked-by? Sorry, I meant Acked-by. K. Y > > You cannot sign off on a patch from > someone else which you are not picking up and transporting it further. > > Thanks, > > tglx
RE: [PATCH] hyperv: a potential NULL pointer dereference
> -Original Message- > From: Kangjie Lu > Sent: Wednesday, March 13, 2019 10:47 PM > To: k...@umn.edu > Cc: pakki...@umn.edu; KY Srinivasan ; Haiyang Zhang > ; Stephen Hemminger > ; Sasha Levin ; Thomas > Gleixner ; Ingo Molnar ; Borislav > Petkov ; H. Peter Anvin ; x...@kernel.org; > linux-hyp...@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: [PATCH] hyperv: a potential NULL pointer dereference > > In case alloc_page, the fix returns -ENOMEM to avoid the potential > NULL pointer dereference. > Thanks. > Signed-off-by: Kangjie Lu Signed-off-by: K. Y. Srinivasan > --- > arch/x86/hyperv/hv_init.c | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c > index 7abb09e2eeb8..dfdb4ce1ae9c 100644 > --- a/arch/x86/hyperv/hv_init.c > +++ b/arch/x86/hyperv/hv_init.c > @@ -102,9 +102,13 @@ static int hv_cpu_init(unsigned int cpu) > u64 msr_vp_index; > struct hv_vp_assist_page **hvp = > _vp_assist_page[smp_processor_id()]; > void **input_arg; > + struct page *pg; > > input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg); > - *input_arg = page_address(alloc_page(GFP_KERNEL)); > + pg = alloc_page(GFP_KERNEL); > + if (unlikely(!pg)) > + return -ENOMEM; > + *input_arg = page_address(pg); > > hv_get_vp_index(msr_vp_index); > > -- > 2.17.1
RE: [PATCH] [repost] Drivers: hv: vmbus: Offload the handling of channels to two workqueues
> -Original Message- > From: Dexuan Cui > Sent: Thursday, November 29, 2018 12:17 AM > To: gre...@linuxfoundation.org > Cc: KY Srinivasan ; Haiyang Zhang > ; Stephen Hemminger > ; linux-kernel@vger.kernel.org; > de...@linuxdriverproject.org; a...@canonical.com; vkuznets > ; o...@aepfle.de; jasow...@redhat.com; Michael > Kelley > Subject: RE: [PATCH] [repost] Drivers: hv: vmbus: Offload the handling of > channels to two workqueues > > > From: gre...@linuxfoundation.org > > Sent: Wednesday, November 28, 2018 11:45 PM > > > > > > There is no change in this repost. I just rebased this patch to today's > > > char-misc's char-misc-next branch. Previously KY posted the patch with > his > > > Signed-off-by (which is kept in this repost), but there was a conflict > > > issue. > > > > > > Note: the patch can't be cleanly applied to char-misc's char-misc-linus > branch > > -- > > > to do that, we need to cherry-pick the supporting patch first: > > > 4d3c5c69191f ("Drivers: hv: vmbus: Remove the useless API > > vmbus_get_outgoing_channel()") > > > > That is not going to work for the obvious reason that this dependant > > patch is not going to be merged into 4.20-final. > > It looks the dependent patch (4d3c5c69191f) is going to miss the v4.20 > release. > This is not a big issue, as the dependent patch isn't really important. > > > So, what do you expect us to do here? The only way this can be accepted > > is to have it go into my -next branch, which means it will show up in > > 4.21-rc1, is that ok? > > Is there any chance for this patch ("Drivers: hv: vmbus: Offload the handling > ...") to > go into v4.20? > > If yes, I can quickly do a rebase to char-misc's char-misc-linus branch, > because actually the conflict can be very easily fixed. And I can help to fix > any > conflict when the dependent patch is backported to v4.20.1. This patch fixes an important bug while the patch this depends on is not critical. I suggest we revert the patch that this patch depends on and we can submit a new version of this patch that can go in now - into 4.20 release. K. Y
RE: [PATCH] [repost] Drivers: hv: vmbus: Offload the handling of channels to two workqueues
> -Original Message- > From: Dexuan Cui > Sent: Thursday, November 29, 2018 12:17 AM > To: gre...@linuxfoundation.org > Cc: KY Srinivasan ; Haiyang Zhang > ; Stephen Hemminger > ; linux-kernel@vger.kernel.org; > de...@linuxdriverproject.org; a...@canonical.com; vkuznets > ; o...@aepfle.de; jasow...@redhat.com; Michael > Kelley > Subject: RE: [PATCH] [repost] Drivers: hv: vmbus: Offload the handling of > channels to two workqueues > > > From: gre...@linuxfoundation.org > > Sent: Wednesday, November 28, 2018 11:45 PM > > > > > > There is no change in this repost. I just rebased this patch to today's > > > char-misc's char-misc-next branch. Previously KY posted the patch with > his > > > Signed-off-by (which is kept in this repost), but there was a conflict > > > issue. > > > > > > Note: the patch can't be cleanly applied to char-misc's char-misc-linus > branch > > -- > > > to do that, we need to cherry-pick the supporting patch first: > > > 4d3c5c69191f ("Drivers: hv: vmbus: Remove the useless API > > vmbus_get_outgoing_channel()") > > > > That is not going to work for the obvious reason that this dependant > > patch is not going to be merged into 4.20-final. > > It looks the dependent patch (4d3c5c69191f) is going to miss the v4.20 > release. > This is not a big issue, as the dependent patch isn't really important. > > > So, what do you expect us to do here? The only way this can be accepted > > is to have it go into my -next branch, which means it will show up in > > 4.21-rc1, is that ok? > > Is there any chance for this patch ("Drivers: hv: vmbus: Offload the handling > ...") to > go into v4.20? > > If yes, I can quickly do a rebase to char-misc's char-misc-linus branch, > because actually the conflict can be very easily fixed. And I can help to fix > any > conflict when the dependent patch is backported to v4.20.1. This patch fixes an important bug while the patch this depends on is not critical. I suggest we revert the patch that this patch depends on and we can submit a new version of this patch that can go in now - into 4.20 release. K. Y
RE: [PATCH 2/2] Drivers: hv: vmbus: offload the handling of channels to two workqueues
> -Original Message- > From: Greg KH > Sent: Monday, November 26, 2018 11:35 AM > To: KY Srinivasan > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org; > o...@aepfle.de; a...@canonical.com; jasow...@redhat.com; Stephen > Hemminger ; Michael Kelley > ; vkuznets ; Haiyang > Zhang ; sta...@vger.kernel.org > Subject: Re: [PATCH 2/2] Drivers: hv: vmbus: offload the handling of channels > to two workqueues > > On Mon, Nov 26, 2018 at 02:29:57AM +, k...@linuxonhyperv.com wrote: > > From: Dexuan Cui > > > > vmbus_process_offer() mustn't call channel->sc_creation_callback() > > directly for sub-channels, because sc_creation_callback() -> > > vmbus_open() may never get the host's response to the > > OPEN_CHANNEL message (the host may rescind a channel at any time, > > e.g. in the case of hot removing a NIC), and vmbus_onoffer_rescind() > > may not wake up the vmbus_open() as it's blocked due to a non-zero > > vmbus_connection.offer_in_progress, and finally we have a deadlock. > > > > The above is also true for primary channels, if the related device > > drivers use sync probing mode by default. > > > > And, usually the handling of primary channels and sub-channels can > > depend on each other, so we should offload them to different > > workqueues to avoid possible deadlock, e.g. in sync-probing mode, > > NIC1's netvsc_subchan_work() can race with NIC2's netvsc_probe() -> > > rtnl_lock(), and causes deadlock: the former gets the rtnl_lock > > and waits for all the sub-channels to appear, but the latter > > can't get the rtnl_lock and this blocks the handling of sub-channels. > > > > The patch can fix the multiple-NIC deadlock described above for > > v3.x kernels (e.g. RHEL 7.x) which don't support async-probing > > of devices, and v4.4, v4.9, v4.14 and v4.18 which support async-probing > > but don't enable async-probing for Hyper-V drivers (yet). > > > > The patch can also fix the hang issue in sub-channel's handling described > > above for all versions of kernels, including v4.19 and v4.20-rc3. > > > > So the patch should be applied to all the existing kernels. > > > > Fixes: 8195b1396ec8 ("hv_netvsc: fix deadlock on hotplug") > > Cc: sta...@vger.kernel.org > > Cc: Stephen Hemminger > > Cc: K. Y. Srinivasan > > Cc: Haiyang Zhang > > Signed-off-by: Dexuan Cui > > Signed-off-by: K. Y. Srinivasan > > --- > > drivers/hv/channel_mgmt.c | 188 +- > > > drivers/hv/connection.c | 24 - > > drivers/hv/hyperv_vmbus.h | 7 ++ > > include/linux/hyperv.h| 7 ++ > > 4 files changed, 161 insertions(+), 65 deletions(-) > > As Sasha pointed out, this patch does not even apply :( Sorry about that. These patches applied cleanly on my tree (misc-next). This series is to be applied on top of patch 0001-Drivers-hv-vmbus-Remove-the-useless-API-vmbus_get_ou.patch While the patch 0001-Drivers-hv-vmbus-Remove-the-useless-API-vmbus_get_ou.patch has been committed to the char-misc-testing branch, it is not in the misc-linus branch and that is the reason for this problem. Regards, K. Y >
RE: [PATCH 2/2] Drivers: hv: vmbus: offload the handling of channels to two workqueues
> -Original Message- > From: Greg KH > Sent: Monday, November 26, 2018 11:35 AM > To: KY Srinivasan > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org; > o...@aepfle.de; a...@canonical.com; jasow...@redhat.com; Stephen > Hemminger ; Michael Kelley > ; vkuznets ; Haiyang > Zhang ; sta...@vger.kernel.org > Subject: Re: [PATCH 2/2] Drivers: hv: vmbus: offload the handling of channels > to two workqueues > > On Mon, Nov 26, 2018 at 02:29:57AM +, k...@linuxonhyperv.com wrote: > > From: Dexuan Cui > > > > vmbus_process_offer() mustn't call channel->sc_creation_callback() > > directly for sub-channels, because sc_creation_callback() -> > > vmbus_open() may never get the host's response to the > > OPEN_CHANNEL message (the host may rescind a channel at any time, > > e.g. in the case of hot removing a NIC), and vmbus_onoffer_rescind() > > may not wake up the vmbus_open() as it's blocked due to a non-zero > > vmbus_connection.offer_in_progress, and finally we have a deadlock. > > > > The above is also true for primary channels, if the related device > > drivers use sync probing mode by default. > > > > And, usually the handling of primary channels and sub-channels can > > depend on each other, so we should offload them to different > > workqueues to avoid possible deadlock, e.g. in sync-probing mode, > > NIC1's netvsc_subchan_work() can race with NIC2's netvsc_probe() -> > > rtnl_lock(), and causes deadlock: the former gets the rtnl_lock > > and waits for all the sub-channels to appear, but the latter > > can't get the rtnl_lock and this blocks the handling of sub-channels. > > > > The patch can fix the multiple-NIC deadlock described above for > > v3.x kernels (e.g. RHEL 7.x) which don't support async-probing > > of devices, and v4.4, v4.9, v4.14 and v4.18 which support async-probing > > but don't enable async-probing for Hyper-V drivers (yet). > > > > The patch can also fix the hang issue in sub-channel's handling described > > above for all versions of kernels, including v4.19 and v4.20-rc3. > > > > So the patch should be applied to all the existing kernels. > > > > Fixes: 8195b1396ec8 ("hv_netvsc: fix deadlock on hotplug") > > Cc: sta...@vger.kernel.org > > Cc: Stephen Hemminger > > Cc: K. Y. Srinivasan > > Cc: Haiyang Zhang > > Signed-off-by: Dexuan Cui > > Signed-off-by: K. Y. Srinivasan > > --- > > drivers/hv/channel_mgmt.c | 188 +- > > > drivers/hv/connection.c | 24 - > > drivers/hv/hyperv_vmbus.h | 7 ++ > > include/linux/hyperv.h| 7 ++ > > 4 files changed, 161 insertions(+), 65 deletions(-) > > As Sasha pointed out, this patch does not even apply :( Sorry about that. These patches applied cleanly on my tree (misc-next). This series is to be applied on top of patch 0001-Drivers-hv-vmbus-Remove-the-useless-API-vmbus_get_ou.patch While the patch 0001-Drivers-hv-vmbus-Remove-the-useless-API-vmbus_get_ou.patch has been committed to the char-misc-testing branch, it is not in the misc-linus branch and that is the reason for this problem. Regards, K. Y >
RE: [PATCH 1/4] arm64: hyperv: Add core Hyper-V include files
> -Original Message- > From: Greg KH > Sent: Monday, November 26, 2018 11:18 AM > To: KY Srinivasan > Cc: will.dea...@arm.com; catalin.mari...@armm.com; > mark.rutl...@arm.com; marc.zyng...@arm.com; linux-arm- > ker...@lists.infradead.org; linux-kernel@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > jasow...@redhat.com; Stephen Hemminger ; > Michael Kelley ; vkuznets > ; Michael Kelley > Subject: Re: [PATCH 1/4] arm64: hyperv: Add core Hyper-V include files > > On Thu, Nov 22, 2018 at 03:10:56AM +, k...@linuxonhyperv.com wrote: > > --- /dev/null > > +++ b/arch/arm64/include/asm/hyperv-tlfs.h > > @@ -0,0 +1,338 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > + > > +/* > > + * This file contains definitions from the Hyper-V Hypervisor Top-Level > > + * Functional Specification (TLFS): > > + * > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs. > microsoft.com%2Fen-us%2Fvirtualization%2Fhyper-v-on- > windows%2Freference%2Ftlfsdata=02%7C01%7Ckys%40microsoft.co > m%7Cce3a8662cbbc4afe157508d653d3eadf%7C72f988bf86f141af91ab2d7cd0 > 11db47%7C1%7C0%7C636788566988323167sdata=VJHHMNbpOdAANd > DNUD3UMuR8SkbBGOew7uy%2B%2BUCS2sE%3Dreserved=0 > > "interesting" url :( > > Care to use a real one? Will fix and resend. K. Y > > thanks, > > greg k-h
RE: [PATCH 1/4] arm64: hyperv: Add core Hyper-V include files
> -Original Message- > From: Greg KH > Sent: Monday, November 26, 2018 11:18 AM > To: KY Srinivasan > Cc: will.dea...@arm.com; catalin.mari...@armm.com; > mark.rutl...@arm.com; marc.zyng...@arm.com; linux-arm- > ker...@lists.infradead.org; linux-kernel@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > jasow...@redhat.com; Stephen Hemminger ; > Michael Kelley ; vkuznets > ; Michael Kelley > Subject: Re: [PATCH 1/4] arm64: hyperv: Add core Hyper-V include files > > On Thu, Nov 22, 2018 at 03:10:56AM +, k...@linuxonhyperv.com wrote: > > --- /dev/null > > +++ b/arch/arm64/include/asm/hyperv-tlfs.h > > @@ -0,0 +1,338 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > + > > +/* > > + * This file contains definitions from the Hyper-V Hypervisor Top-Level > > + * Functional Specification (TLFS): > > + * > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs. > microsoft.com%2Fen-us%2Fvirtualization%2Fhyper-v-on- > windows%2Freference%2Ftlfsdata=02%7C01%7Ckys%40microsoft.co > m%7Cce3a8662cbbc4afe157508d653d3eadf%7C72f988bf86f141af91ab2d7cd0 > 11db47%7C1%7C0%7C636788566988323167sdata=VJHHMNbpOdAANd > DNUD3UMuR8SkbBGOew7uy%2B%2BUCS2sE%3Dreserved=0 > > "interesting" url :( > > Care to use a real one? Will fix and resend. K. Y > > thanks, > > greg k-h
RE: [PATCH 0/4] Hyper-V: Enable Linux guests on Hyper-V on ARM64
> -Original Message- > From: k...@linuxonhyperv.com > Sent: Wednesday, November 21, 2018 6:51 PM > To: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > jasow...@redhat.com; Stephen Hemminger ; > Michael Kelley ; vkuznets > > Cc: KY Srinivasan > Subject: [PATCH 0/4] Hyper-V: Enable Linux guests on Hyper-V on ARM64 > > From: "K. Y. Srinivasan" > > This series enables Linux guests running on Hyper-V on ARM64 > hardware. New ARM64-specific code in arch/arm64/hyperv initializes > Hyper-V, including its synthetic clocks and hypercall mechanism. > Existing architecture independent drivers for Hyper-V's VMbus and > synthetic devices just work when built for ARM64. Hyper-V code is > built and included in the image and modules only if CONFIG_HYPERV > is enabled. > > The four patches are organized as follows: > 1) Add include files that define the Hyper-V interface as >described in the Hyper-V Top Level Functional Spec (TLFS), plus >additional definitions specific to Linux running on Hyper-V. > > 2) Add core Hyper-V support on ARM64, including hypercalls, >synthetic clock initialization, and interrupt handlers. > > 3) Update the existing VMbus driver to generalize interrupt >management across x86/x64 and ARM64. > > 4) Make CONFIG_HYPERV selectable on ARM64 in addition to x86/x64. > > Some areas of Linux guests on Hyper-V on ARM64 are a work- > in-progress, primarily due to work still being done in Hyper-V: > > * Hyper-V on ARM64 currently runs with a 4 Kbyte page size, and only > supports guests with a 4 Kbyte page size. Because Hyper-V uses > shared pages to communicate between the guest and the hypervisor, > there are open design decisions on the page size to use when > the guest is using 16K/64K pages. Once those issues are > resolved and Hyper-V fully supports 16K/64K guest pages, changes > may be needed in the Linux drivers for Hyper-V synthetic devices. > > * Hyper-V on ARM64 does not currently support mapping PCI devices > into the guest address space. The Hyper-V PCI driver at > drivers/pci/host/pci-hyperv.c has x86/x64-specific code and is > not being built for ARM64. > > In a few cases, terminology from the x86/x64 world has been carried > over into the ARM64 code ("MSR", "TSC"). Hyper-V still uses the > x86/x64 terminology and has not replaced it with something more > generic, so the code uses the Hyper-V terminology. This will be > fixed when Hyper-V updates the usage in the TLFS. > > > Michael Kelley (4): > arm64: hyperv: Add core Hyper-V include files > arm64: hyperv: Add support for Hyper-V as a hypervisor > Drivers: hv: vmbus: Add hooks for per-CPU IRQ > Drivers: hv: Enable CONFIG_HYPERV on ARM64 > > MAINTAINERS | 4 + > arch/arm64/Makefile | 1 + > arch/arm64/hyperv/Makefile | 2 + > arch/arm64/hyperv/hv_hvc.S | 54 > arch/arm64/hyperv/hv_init.c | 441 +++ > arch/arm64/hyperv/mshyperv.c | 178 +++ > arch/arm64/include/asm/hyperv-tlfs.h | 338 > arch/arm64/include/asm/mshyperv.h| 116 +++ > arch/x86/include/asm/mshyperv.h | 4 + > drivers/hv/Kconfig | 3 +- > drivers/hv/hv.c | 2 + > include/asm-generic/mshyperv.h | 240 +++ > 12 files changed, 1382 insertions(+), 1 deletion(-) > create mode 100644 arch/arm64/hyperv/Makefile > create mode 100644 arch/arm64/hyperv/hv_hvc.S > create mode 100644 arch/arm64/hyperv/hv_init.c > create mode 100644 arch/arm64/hyperv/mshyperv.c > create mode 100644 arch/arm64/include/asm/hyperv-tlfs.h > create mode 100644 arch/arm64/include/asm/mshyperv.h > create mode 100644 include/asm-generic/mshyperv.h > Greg, Please drop this. I will be resending shortly. K. Y > -- > 2.19.1
RE: [PATCH 0/4] Hyper-V: Enable Linux guests on Hyper-V on ARM64
> -Original Message- > From: k...@linuxonhyperv.com > Sent: Wednesday, November 21, 2018 6:51 PM > To: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > jasow...@redhat.com; Stephen Hemminger ; > Michael Kelley ; vkuznets > > Cc: KY Srinivasan > Subject: [PATCH 0/4] Hyper-V: Enable Linux guests on Hyper-V on ARM64 > > From: "K. Y. Srinivasan" > > This series enables Linux guests running on Hyper-V on ARM64 > hardware. New ARM64-specific code in arch/arm64/hyperv initializes > Hyper-V, including its synthetic clocks and hypercall mechanism. > Existing architecture independent drivers for Hyper-V's VMbus and > synthetic devices just work when built for ARM64. Hyper-V code is > built and included in the image and modules only if CONFIG_HYPERV > is enabled. > > The four patches are organized as follows: > 1) Add include files that define the Hyper-V interface as >described in the Hyper-V Top Level Functional Spec (TLFS), plus >additional definitions specific to Linux running on Hyper-V. > > 2) Add core Hyper-V support on ARM64, including hypercalls, >synthetic clock initialization, and interrupt handlers. > > 3) Update the existing VMbus driver to generalize interrupt >management across x86/x64 and ARM64. > > 4) Make CONFIG_HYPERV selectable on ARM64 in addition to x86/x64. > > Some areas of Linux guests on Hyper-V on ARM64 are a work- > in-progress, primarily due to work still being done in Hyper-V: > > * Hyper-V on ARM64 currently runs with a 4 Kbyte page size, and only > supports guests with a 4 Kbyte page size. Because Hyper-V uses > shared pages to communicate between the guest and the hypervisor, > there are open design decisions on the page size to use when > the guest is using 16K/64K pages. Once those issues are > resolved and Hyper-V fully supports 16K/64K guest pages, changes > may be needed in the Linux drivers for Hyper-V synthetic devices. > > * Hyper-V on ARM64 does not currently support mapping PCI devices > into the guest address space. The Hyper-V PCI driver at > drivers/pci/host/pci-hyperv.c has x86/x64-specific code and is > not being built for ARM64. > > In a few cases, terminology from the x86/x64 world has been carried > over into the ARM64 code ("MSR", "TSC"). Hyper-V still uses the > x86/x64 terminology and has not replaced it with something more > generic, so the code uses the Hyper-V terminology. This will be > fixed when Hyper-V updates the usage in the TLFS. > > > Michael Kelley (4): > arm64: hyperv: Add core Hyper-V include files > arm64: hyperv: Add support for Hyper-V as a hypervisor > Drivers: hv: vmbus: Add hooks for per-CPU IRQ > Drivers: hv: Enable CONFIG_HYPERV on ARM64 > > MAINTAINERS | 4 + > arch/arm64/Makefile | 1 + > arch/arm64/hyperv/Makefile | 2 + > arch/arm64/hyperv/hv_hvc.S | 54 > arch/arm64/hyperv/hv_init.c | 441 +++ > arch/arm64/hyperv/mshyperv.c | 178 +++ > arch/arm64/include/asm/hyperv-tlfs.h | 338 > arch/arm64/include/asm/mshyperv.h| 116 +++ > arch/x86/include/asm/mshyperv.h | 4 + > drivers/hv/Kconfig | 3 +- > drivers/hv/hv.c | 2 + > include/asm-generic/mshyperv.h | 240 +++ > 12 files changed, 1382 insertions(+), 1 deletion(-) > create mode 100644 arch/arm64/hyperv/Makefile > create mode 100644 arch/arm64/hyperv/hv_hvc.S > create mode 100644 arch/arm64/hyperv/hv_init.c > create mode 100644 arch/arm64/hyperv/mshyperv.c > create mode 100644 arch/arm64/include/asm/hyperv-tlfs.h > create mode 100644 arch/arm64/include/asm/mshyperv.h > create mode 100644 include/asm-generic/mshyperv.h > Greg, Please drop this. I will be resending shortly. K. Y > -- > 2.19.1
RE: [PATCH V2 3/5] Drivers: hv: kvp: Fix the recent regression caused by incorrect clean-up
> -Original Message- > From: gre...@linuxfoundation.org > Sent: Thursday, November 1, 2018 11:57 AM > To: Dexuan Cui > Cc: Michael Kelley ; KY Srinivasan > ; linux-kernel@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > jasow...@redhat.com; Stephen Hemminger ; > vkuznets ; Sasha Levin > ; Haiyang Zhang > ; sta...@vger.kernel.org > Subject: Re: [PATCH V2 3/5] Drivers: hv: kvp: Fix the recent regression caused > by incorrect clean-up > > On Wed, Oct 31, 2018 at 11:23:54PM +, Dexuan Cui wrote: > > > From: Michael Kelley > > > Sent: Wednesday, October 24, 2018 08:38 > > > From: k...@linuxonhyperv.com Sent: > Wednesday, > > > October 17, 2018 10:10 PM > > > > From: Dexuan Cui > > > > > > > > In kvp_send_key(), we do need call process_ib_ipinfo() if > > > > message->kvp_hdr.operation is KVP_OP_GET_IP_INFO, because it > turns out > > > > the userland hv_kvp_daemon needs the info of operation, adapter_id > and > > > > addr_family. With the incorrect fc62c3b1977d, the host can't get the > > > > VM's IP via KVP. > > > > > > > > And, fc62c3b1977d added a "break;", but actually forgot to initialize > > > > the key_size/value in the case of KVP_OP_SET, so the default key_size > of > > > > 0 is passed to the kvp daemon, and the pool files > > > > /var/lib/hyperv/.kvp_pool_* can't be updated. > > > > > > > > This patch effectively rolls back the previous fc62c3b1977d, and > > > > correctly fixes the "this statement may fall through" warnings. > > > > > > > > This patch is tested on WS 2012 R2 and 2016. > > > > > > > > Fixes: fc62c3b1977d ("Drivers: hv: kvp: Fix two "this statement may fall > > > through" warnings") > > > > Signed-off-by: Dexuan Cui > > > > Cc: K. Y. Srinivasan > > > > Cc: Haiyang Zhang > > > > Cc: Stephen Hemminger > > > > Cc: > > > > Signed-off-by: K. Y. Srinivasan > > > > --- > > > > drivers/hv/hv_kvp.c | 26 ++ > > > > 1 file changed, 22 insertions(+), 4 deletions(-) > > > > > > > Reviewed-by: Michael Kelley > > > > Hi Greg, > > Can you please take a look at this patch? Greg, I have already signed-off on this patch. K. Y > > Nope, I'm not the hv maintainer, they need to look at this and ack it, > not me :) > > greg k-h
RE: [PATCH V2 3/5] Drivers: hv: kvp: Fix the recent regression caused by incorrect clean-up
> -Original Message- > From: gre...@linuxfoundation.org > Sent: Thursday, November 1, 2018 11:57 AM > To: Dexuan Cui > Cc: Michael Kelley ; KY Srinivasan > ; linux-kernel@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > jasow...@redhat.com; Stephen Hemminger ; > vkuznets ; Sasha Levin > ; Haiyang Zhang > ; sta...@vger.kernel.org > Subject: Re: [PATCH V2 3/5] Drivers: hv: kvp: Fix the recent regression caused > by incorrect clean-up > > On Wed, Oct 31, 2018 at 11:23:54PM +, Dexuan Cui wrote: > > > From: Michael Kelley > > > Sent: Wednesday, October 24, 2018 08:38 > > > From: k...@linuxonhyperv.com Sent: > Wednesday, > > > October 17, 2018 10:10 PM > > > > From: Dexuan Cui > > > > > > > > In kvp_send_key(), we do need call process_ib_ipinfo() if > > > > message->kvp_hdr.operation is KVP_OP_GET_IP_INFO, because it > turns out > > > > the userland hv_kvp_daemon needs the info of operation, adapter_id > and > > > > addr_family. With the incorrect fc62c3b1977d, the host can't get the > > > > VM's IP via KVP. > > > > > > > > And, fc62c3b1977d added a "break;", but actually forgot to initialize > > > > the key_size/value in the case of KVP_OP_SET, so the default key_size > of > > > > 0 is passed to the kvp daemon, and the pool files > > > > /var/lib/hyperv/.kvp_pool_* can't be updated. > > > > > > > > This patch effectively rolls back the previous fc62c3b1977d, and > > > > correctly fixes the "this statement may fall through" warnings. > > > > > > > > This patch is tested on WS 2012 R2 and 2016. > > > > > > > > Fixes: fc62c3b1977d ("Drivers: hv: kvp: Fix two "this statement may fall > > > through" warnings") > > > > Signed-off-by: Dexuan Cui > > > > Cc: K. Y. Srinivasan > > > > Cc: Haiyang Zhang > > > > Cc: Stephen Hemminger > > > > Cc: > > > > Signed-off-by: K. Y. Srinivasan > > > > --- > > > > drivers/hv/hv_kvp.c | 26 ++ > > > > 1 file changed, 22 insertions(+), 4 deletions(-) > > > > > > > Reviewed-by: Michael Kelley > > > > Hi Greg, > > Can you please take a look at this patch? Greg, I have already signed-off on this patch. K. Y > > Nope, I'm not the hv maintainer, they need to look at this and ack it, > not me :) > > greg k-h
RE: [PATCH 3/5] Drivers: hv: kvp: Fix the recent regression caused by incorrect clean-up
> -Original Message- > From: Greg KH > Sent: Tuesday, October 16, 2018 10:07 PM > To: KY Srinivasan > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org; > o...@aepfle.de; a...@canonical.com; jasow...@redhat.com; Stephen > Hemminger ; Michael Kelley (EOSG) > ; vkuznets ; > Haiyang Zhang ; sta...@vger.kernel.org > Subject: Re: [PATCH 3/5] Drivers: hv: kvp: Fix the recent regression caused by > incorrect clean-up > > On Wed, Oct 17, 2018 at 03:14:04AM +, k...@linuxonhyperv.com wrote: > > From: Dexuan Cui > > > > In kvp_send_key(), we do need call process_ib_ipinfo() if > > message->kvp_hdr.operation is KVP_OP_GET_IP_INFO, because it turns > out > > the userland hv_kvp_daemon needs the info of operation, adapter_id and > > addr_family. With the incorrect fc62c3b1977d, the host can't get the > > VM's IP via KVP. > > > > And, fc62c3b1977d added a "break;", but actually forgot to initialize > > the key_size/value in the case of KVP_OP_SET, so the default key_size of > > 0 is passed to the kvp daemon, and the pool files > > /var/lib/hyperv/.kvp_pool_* can't be updated. > > > > This patch effectively rolls back the previous fc62c3b1977d, and > > correctly fixes the "this statement may fall through" warnings. > > > > This patch is tested on WS 2012 R2 and 2016. > > > > Fixes: fc62c3b1977d ("Drivers: hv: kvp: Fix two "this statement may fall > through" warnings") > > Signed-off-by: Dexuan Cui > > Cc: K. Y. Srinivasan > > Cc: Haiyang Zhang > > Cc: Stephen Hemminger > > Cc: > > Signed-off-by: K. Y. Srinivasan > > --- > > drivers/hv/hv_kvp.c | 26 ++ > > 1 file changed, 22 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c > > index a7513a8a8e37..9fbb15c62c6c 100644 > > --- a/drivers/hv/hv_kvp.c > > +++ b/drivers/hv/hv_kvp.c > > @@ -353,6 +353,9 @@ static void process_ib_ipinfo(void *in_msg, void > *out_msg, int op) > > > > out->body.kvp_ip_val.dhcp_enabled = in- > >kvp_ip_val.dhcp_enabled; > > > > + __attribute__ ((fallthrough)); > > The comment should be sufficient for this, right? I haven't seen many > uses of this attribute before, how common is it? Yes; a common should be sufficient. > > > > + > > + case KVP_OP_GET_IP_INFO: > > utf16s_to_utf8s((wchar_t *)in->kvp_ip_val.adapter_id, > > MAX_ADAPTER_ID_SIZE, > > UTF16_LITTLE_ENDIAN, > > @@ -405,7 +408,11 @@ kvp_send_key(struct work_struct *dummy) > > process_ib_ipinfo(in_msg, message, KVP_OP_SET_IP_INFO); > > break; > > case KVP_OP_GET_IP_INFO: > > - /* We only need to pass on message->kvp_hdr.operation. > */ > > + /* > > +* We only need to pass on the info of operation, adapter_id > > +* and addr_family to the userland kvp daemon. > > +*/ > > + process_ib_ipinfo(in_msg, message, > KVP_OP_GET_IP_INFO); > > break; > > case KVP_OP_SET: > > switch (in_msg->body.kvp_set.data.value_type) { > > @@ -446,9 +453,9 @@ kvp_send_key(struct work_struct *dummy) > > > > } > > > > - break; > > - > > - case KVP_OP_GET: > > + /* > > +* The key is always a string - utf16 encoding. > > +*/ > > message->body.kvp_set.data.key_size = > > utf16s_to_utf8s( > > (wchar_t *)in_msg->body.kvp_set.data.key, > > @@ -456,6 +463,17 @@ kvp_send_key(struct work_struct *dummy) > > UTF16_LITTLE_ENDIAN, > > message->body.kvp_set.data.key, > > HV_KVP_EXCHANGE_MAX_KEY_SIZE - 1) + 1; > > + > > + break; > > + > > + case KVP_OP_GET: > > + message->body.kvp_get.data.key_size = > > + utf16s_to_utf8s( > > + (wchar_t *)in_msg->body.kvp_get.data.key, > > + in_msg->body.kvp_get.data.key_size, > > + UTF16_LITTLE_ENDIAN, > > + message->body.kvp_get.data.key, > > + HV_KVP_EXCHANGE_MAX_KEY_SIZE - 1) + 1; > > Worst indentation ever :( > > Yeah, I know it follows the others above it, but you should reconsider > it in the future... > > thanks, > > greg k-h
RE: [PATCH 3/5] Drivers: hv: kvp: Fix the recent regression caused by incorrect clean-up
> -Original Message- > From: Greg KH > Sent: Tuesday, October 16, 2018 10:07 PM > To: KY Srinivasan > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org; > o...@aepfle.de; a...@canonical.com; jasow...@redhat.com; Stephen > Hemminger ; Michael Kelley (EOSG) > ; vkuznets ; > Haiyang Zhang ; sta...@vger.kernel.org > Subject: Re: [PATCH 3/5] Drivers: hv: kvp: Fix the recent regression caused by > incorrect clean-up > > On Wed, Oct 17, 2018 at 03:14:04AM +, k...@linuxonhyperv.com wrote: > > From: Dexuan Cui > > > > In kvp_send_key(), we do need call process_ib_ipinfo() if > > message->kvp_hdr.operation is KVP_OP_GET_IP_INFO, because it turns > out > > the userland hv_kvp_daemon needs the info of operation, adapter_id and > > addr_family. With the incorrect fc62c3b1977d, the host can't get the > > VM's IP via KVP. > > > > And, fc62c3b1977d added a "break;", but actually forgot to initialize > > the key_size/value in the case of KVP_OP_SET, so the default key_size of > > 0 is passed to the kvp daemon, and the pool files > > /var/lib/hyperv/.kvp_pool_* can't be updated. > > > > This patch effectively rolls back the previous fc62c3b1977d, and > > correctly fixes the "this statement may fall through" warnings. > > > > This patch is tested on WS 2012 R2 and 2016. > > > > Fixes: fc62c3b1977d ("Drivers: hv: kvp: Fix two "this statement may fall > through" warnings") > > Signed-off-by: Dexuan Cui > > Cc: K. Y. Srinivasan > > Cc: Haiyang Zhang > > Cc: Stephen Hemminger > > Cc: > > Signed-off-by: K. Y. Srinivasan > > --- > > drivers/hv/hv_kvp.c | 26 ++ > > 1 file changed, 22 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c > > index a7513a8a8e37..9fbb15c62c6c 100644 > > --- a/drivers/hv/hv_kvp.c > > +++ b/drivers/hv/hv_kvp.c > > @@ -353,6 +353,9 @@ static void process_ib_ipinfo(void *in_msg, void > *out_msg, int op) > > > > out->body.kvp_ip_val.dhcp_enabled = in- > >kvp_ip_val.dhcp_enabled; > > > > + __attribute__ ((fallthrough)); > > The comment should be sufficient for this, right? I haven't seen many > uses of this attribute before, how common is it? Yes; a common should be sufficient. > > > > + > > + case KVP_OP_GET_IP_INFO: > > utf16s_to_utf8s((wchar_t *)in->kvp_ip_val.adapter_id, > > MAX_ADAPTER_ID_SIZE, > > UTF16_LITTLE_ENDIAN, > > @@ -405,7 +408,11 @@ kvp_send_key(struct work_struct *dummy) > > process_ib_ipinfo(in_msg, message, KVP_OP_SET_IP_INFO); > > break; > > case KVP_OP_GET_IP_INFO: > > - /* We only need to pass on message->kvp_hdr.operation. > */ > > + /* > > +* We only need to pass on the info of operation, adapter_id > > +* and addr_family to the userland kvp daemon. > > +*/ > > + process_ib_ipinfo(in_msg, message, > KVP_OP_GET_IP_INFO); > > break; > > case KVP_OP_SET: > > switch (in_msg->body.kvp_set.data.value_type) { > > @@ -446,9 +453,9 @@ kvp_send_key(struct work_struct *dummy) > > > > } > > > > - break; > > - > > - case KVP_OP_GET: > > + /* > > +* The key is always a string - utf16 encoding. > > +*/ > > message->body.kvp_set.data.key_size = > > utf16s_to_utf8s( > > (wchar_t *)in_msg->body.kvp_set.data.key, > > @@ -456,6 +463,17 @@ kvp_send_key(struct work_struct *dummy) > > UTF16_LITTLE_ENDIAN, > > message->body.kvp_set.data.key, > > HV_KVP_EXCHANGE_MAX_KEY_SIZE - 1) + 1; > > + > > + break; > > + > > + case KVP_OP_GET: > > + message->body.kvp_get.data.key_size = > > + utf16s_to_utf8s( > > + (wchar_t *)in_msg->body.kvp_get.data.key, > > + in_msg->body.kvp_get.data.key_size, > > + UTF16_LITTLE_ENDIAN, > > + message->body.kvp_get.data.key, > > + HV_KVP_EXCHANGE_MAX_KEY_SIZE - 1) + 1; > > Worst indentation ever :( > > Yeah, I know it follows the others above it, but you should reconsider > it in the future... > > thanks, > > greg k-h
RE: [PATCH 1/5] Drivers: hv: vmbus: Get rid of unnecessary state in hv_context
> -Original Message- > From: Greg KH > Sent: Tuesday, October 16, 2018 10:05 PM > To: KY Srinivasan > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org; > o...@aepfle.de; a...@canonical.com; jasow...@redhat.com; Stephen > Hemminger ; Michael Kelley (EOSG) > ; vkuznets > Subject: Re: [PATCH 1/5] Drivers: hv: vmbus: Get rid of unnecessary state in > hv_context > > On Wed, Oct 17, 2018 at 03:14:02AM +, k...@linuxonhyperv.com wrote: > > From: "K. Y. Srinivasan" > > > > Currently we are replicating state in struct hv_context that is unnecessary > > - > > this state can be retrieved from the hypervisor. Furthermore, this is a per- > cpu > > state that is being maintained as a global state in struct hv_context. > > Get rid of this state in struct hv_context. > > > > Reply-To: k...@microsoft.com > > Why is this here? No sure how this happened; will fix it and resend. K. Y > > greg k-h
RE: [PATCH 1/5] Drivers: hv: vmbus: Get rid of unnecessary state in hv_context
> -Original Message- > From: Greg KH > Sent: Tuesday, October 16, 2018 10:05 PM > To: KY Srinivasan > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org; > o...@aepfle.de; a...@canonical.com; jasow...@redhat.com; Stephen > Hemminger ; Michael Kelley (EOSG) > ; vkuznets > Subject: Re: [PATCH 1/5] Drivers: hv: vmbus: Get rid of unnecessary state in > hv_context > > On Wed, Oct 17, 2018 at 03:14:02AM +, k...@linuxonhyperv.com wrote: > > From: "K. Y. Srinivasan" > > > > Currently we are replicating state in struct hv_context that is unnecessary > > - > > this state can be retrieved from the hypervisor. Furthermore, this is a per- > cpu > > state that is being maintained as a global state in struct hv_context. > > Get rid of this state in struct hv_context. > > > > Reply-To: k...@microsoft.com > > Why is this here? No sure how this happened; will fix it and resend. K. Y > > greg k-h
RE: [PATCH 4/5] Drivers: hv: kvp: Use %u to print U32
> -Original Message- > From: Greg KH > Sent: Tuesday, October 16, 2018 10:04 PM > To: KY Srinivasan > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org; > o...@aepfle.de; a...@canonical.com; jasow...@redhat.com; Stephen > Hemminger ; Michael Kelley (EOSG) > ; vkuznets ; > Haiyang Zhang ; sta...@vger.kernel.org > Subject: Re: [PATCH 4/5] Drivers: hv: kvp: Use %u to print U32 > > On Wed, Oct 17, 2018 at 03:14:05AM +, k...@linuxonhyperv.com wrote: > > From: Dexuan Cui > > > > I didn't find a real issue. Let's just make it consistent with the > > next "case REG_U64:" where %llu is used. > > > > Signed-off-by: Dexuan Cui > > Cc: K. Y. Srinivasan > > Cc: Haiyang Zhang > > Cc: Stephen Hemminger > > Cc: > > Why is this a stable patch? My mistake; I will resend. K. Y > > confused, > > greg k-h
RE: [PATCH 4/5] Drivers: hv: kvp: Use %u to print U32
> -Original Message- > From: Greg KH > Sent: Tuesday, October 16, 2018 10:04 PM > To: KY Srinivasan > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org; > o...@aepfle.de; a...@canonical.com; jasow...@redhat.com; Stephen > Hemminger ; Michael Kelley (EOSG) > ; vkuznets ; > Haiyang Zhang ; sta...@vger.kernel.org > Subject: Re: [PATCH 4/5] Drivers: hv: kvp: Use %u to print U32 > > On Wed, Oct 17, 2018 at 03:14:05AM +, k...@linuxonhyperv.com wrote: > > From: Dexuan Cui > > > > I didn't find a real issue. Let's just make it consistent with the > > next "case REG_U64:" where %llu is used. > > > > Signed-off-by: Dexuan Cui > > Cc: K. Y. Srinivasan > > Cc: Haiyang Zhang > > Cc: Stephen Hemminger > > Cc: > > Why is this a stable patch? My mistake; I will resend. K. Y > > confused, > > greg k-h
RE: [PATCH 2/2] tools: hv: fcopy: set 'error' in case an unknown operation was requested
> -Original Message- > From: Greg KH > Sent: Monday, September 17, 2018 7:28 AM > To: KY Srinivasan > Cc: o...@aepfle.de; Stephen Hemminger ; > jasow...@redhat.com; linux-kernel@vger.kernel.org; Michael Kelley > (EOSG) ; a...@canonical.com; > de...@linuxdriverproject.org; vkuznets > Subject: Re: [PATCH 2/2] tools: hv: fcopy: set 'error' in case an unknown > operation was requested > > On Mon, Sep 17, 2018 at 02:16:48PM +, KY Srinivasan wrote: > > > > > > > -Original Message- > > > From: Greg KH > > > Sent: Sunday, September 16, 2018 9:56 PM > > > To: KY Srinivasan > > > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org; > > > o...@aepfle.de; a...@canonical.com; jasow...@redhat.com; Stephen > > > Hemminger ; Michael Kelley (EOSG) > > > ; vkuznets > > > Subject: Re: [PATCH 2/2] tools: hv: fcopy: set 'error' in case an unknown > > > operation was requested > > > > > > On Mon, Sep 17, 2018 at 04:14:55AM +, k...@linuxonhyperv.com > wrote: > > > > From: Vitaly Kuznetsov > > > > > > > > 'error' variable is left uninitialized in case we see an unknown > > > > operation. > > > > As we don't immediately return and proceed to pwrite() we need to set > it > > > > to something, HV_E_FAIL sounds good enough. > > > > > > > > Signed-off-by: Vitaly Kuznetsov > > > > Signed-off-by: K. Y. Srinivasan > > > > --- > > > > tools/hv/hv_fcopy_daemon.c | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > No need to backport for stable? > > Thanks for pointing this out. Should I resubmit with the stable tag? > > I can do it... :) Thank you! K. Y
RE: [PATCH 2/2] tools: hv: fcopy: set 'error' in case an unknown operation was requested
> -Original Message- > From: Greg KH > Sent: Monday, September 17, 2018 7:28 AM > To: KY Srinivasan > Cc: o...@aepfle.de; Stephen Hemminger ; > jasow...@redhat.com; linux-kernel@vger.kernel.org; Michael Kelley > (EOSG) ; a...@canonical.com; > de...@linuxdriverproject.org; vkuznets > Subject: Re: [PATCH 2/2] tools: hv: fcopy: set 'error' in case an unknown > operation was requested > > On Mon, Sep 17, 2018 at 02:16:48PM +, KY Srinivasan wrote: > > > > > > > -Original Message- > > > From: Greg KH > > > Sent: Sunday, September 16, 2018 9:56 PM > > > To: KY Srinivasan > > > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org; > > > o...@aepfle.de; a...@canonical.com; jasow...@redhat.com; Stephen > > > Hemminger ; Michael Kelley (EOSG) > > > ; vkuznets > > > Subject: Re: [PATCH 2/2] tools: hv: fcopy: set 'error' in case an unknown > > > operation was requested > > > > > > On Mon, Sep 17, 2018 at 04:14:55AM +, k...@linuxonhyperv.com > wrote: > > > > From: Vitaly Kuznetsov > > > > > > > > 'error' variable is left uninitialized in case we see an unknown > > > > operation. > > > > As we don't immediately return and proceed to pwrite() we need to set > it > > > > to something, HV_E_FAIL sounds good enough. > > > > > > > > Signed-off-by: Vitaly Kuznetsov > > > > Signed-off-by: K. Y. Srinivasan > > > > --- > > > > tools/hv/hv_fcopy_daemon.c | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > No need to backport for stable? > > Thanks for pointing this out. Should I resubmit with the stable tag? > > I can do it... :) Thank you! K. Y
RE: [PATCH 2/2] tools: hv: fcopy: set 'error' in case an unknown operation was requested
> -Original Message- > From: Greg KH > Sent: Sunday, September 16, 2018 9:56 PM > To: KY Srinivasan > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org; > o...@aepfle.de; a...@canonical.com; jasow...@redhat.com; Stephen > Hemminger ; Michael Kelley (EOSG) > ; vkuznets > Subject: Re: [PATCH 2/2] tools: hv: fcopy: set 'error' in case an unknown > operation was requested > > On Mon, Sep 17, 2018 at 04:14:55AM +, k...@linuxonhyperv.com wrote: > > From: Vitaly Kuznetsov > > > > 'error' variable is left uninitialized in case we see an unknown operation. > > As we don't immediately return and proceed to pwrite() we need to set it > > to something, HV_E_FAIL sounds good enough. > > > > Signed-off-by: Vitaly Kuznetsov > > Signed-off-by: K. Y. Srinivasan > > --- > > tools/hv/hv_fcopy_daemon.c | 1 + > > 1 file changed, 1 insertion(+) > > No need to backport for stable? Thanks for pointing this out. Should I resubmit with the stable tag? Thanks, K. Y
RE: [PATCH 2/2] tools: hv: fcopy: set 'error' in case an unknown operation was requested
> -Original Message- > From: Greg KH > Sent: Sunday, September 16, 2018 9:56 PM > To: KY Srinivasan > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org; > o...@aepfle.de; a...@canonical.com; jasow...@redhat.com; Stephen > Hemminger ; Michael Kelley (EOSG) > ; vkuznets > Subject: Re: [PATCH 2/2] tools: hv: fcopy: set 'error' in case an unknown > operation was requested > > On Mon, Sep 17, 2018 at 04:14:55AM +, k...@linuxonhyperv.com wrote: > > From: Vitaly Kuznetsov > > > > 'error' variable is left uninitialized in case we see an unknown operation. > > As we don't immediately return and proceed to pwrite() we need to set it > > to something, HV_E_FAIL sounds good enough. > > > > Signed-off-by: Vitaly Kuznetsov > > Signed-off-by: K. Y. Srinivasan > > --- > > tools/hv/hv_fcopy_daemon.c | 1 + > > 1 file changed, 1 insertion(+) > > No need to backport for stable? Thanks for pointing this out. Should I resubmit with the stable tag? Thanks, K. Y
RE: [PATCH v5 4/5] x86/hyper-v: rename ipi_arg_{ex,non_ex} structures
> -Original Message- > From: Paolo Bonzini > Sent: Friday, September 14, 2018 10:37 AM > To: vkuznets ; k...@vger.kernel.org > Cc: Radim Krčmář ; Roman Kagan > ; KY Srinivasan ; Haiyang > Zhang ; Stephen Hemminger > ; Michael Kelley (EOSG) > ; Mohammed Gamal > ; Cathy Avery ; Wanpeng Li > ; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v5 4/5] x86/hyper-v: rename ipi_arg_{ex,non_ex} > structures > > On 27/08/2018 18:48, Vitaly Kuznetsov wrote: > > These structures are going to be used from KVM code so let's make > > their names reflect their Hyper-V origin. > > > > Signed-off-by: Vitaly Kuznetsov > > Reviewed-by: Roman Kagan > > KY, can you ack this patch? Acked-by: K. Y. Srinivasan > > Thanks, > > Paolo > > > --- > > arch/x86/hyperv/hv_apic.c | 8 > > arch/x86/include/asm/hyperv-tlfs.h | 16 +--- > > 2 files changed, 13 insertions(+), 11 deletions(-) > > > > diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c > > index 5b0f613428c2..2c43e3055948 100644 > > --- a/arch/x86/hyperv/hv_apic.c > > +++ b/arch/x86/hyperv/hv_apic.c > > @@ -95,8 +95,8 @@ static void hv_apic_eoi_write(u32 reg, u32 val) > > */ > > static bool __send_ipi_mask_ex(const struct cpumask *mask, int vector) > > { > > - struct ipi_arg_ex **arg; > > - struct ipi_arg_ex *ipi_arg; > > + struct hv_send_ipi_ex **arg; > > + struct hv_send_ipi_ex *ipi_arg; > > unsigned long flags; > > int nr_bank = 0; > > int ret = 1; > > @@ -105,7 +105,7 @@ static bool __send_ipi_mask_ex(const struct > cpumask *mask, int vector) > > return false; > > > > local_irq_save(flags); > > - arg = (struct ipi_arg_ex **)this_cpu_ptr(hyperv_pcpu_input_arg); > > + arg = (struct hv_send_ipi_ex > **)this_cpu_ptr(hyperv_pcpu_input_arg); > > > > ipi_arg = *arg; > > if (unlikely(!ipi_arg)) > > @@ -135,7 +135,7 @@ static bool __send_ipi_mask_ex(const struct > cpumask *mask, int vector) > > static bool __send_ipi_mask(const struct cpumask *mask, int vector) > > { > > int cur_cpu, vcpu; > > - struct ipi_arg_non_ex ipi_arg; > > + struct hv_send_ipi ipi_arg; > > int ret = 1; > > > > trace_hyperv_send_ipi_mask(mask, vector); > > diff --git a/arch/x86/include/asm/hyperv-tlfs.h > b/arch/x86/include/asm/hyperv-tlfs.h > > index e977b6b3a538..00e01d215f74 100644 > > --- a/arch/x86/include/asm/hyperv-tlfs.h > > +++ b/arch/x86/include/asm/hyperv-tlfs.h > > @@ -726,19 +726,21 @@ struct hv_enlightened_vmcs { > > #define HV_STIMER_AUTOENABLE (1ULL << 3) > > #define HV_STIMER_SINT(config) (__u8)(((config) >> 16) & > 0x0F) > > > > -struct ipi_arg_non_ex { > > - u32 vector; > > - u32 reserved; > > - u64 cpu_mask; > > -}; > > - > > struct hv_vpset { > > u64 format; > > u64 valid_bank_mask; > > u64 bank_contents[]; > > }; > > > > -struct ipi_arg_ex { > > +/* HvCallSendSyntheticClusterIpi hypercall */ > > +struct hv_send_ipi { > > + u32 vector; > > + u32 reserved; > > + u64 cpu_mask; > > +}; > > + > > +/* HvCallSendSyntheticClusterIpiEx hypercall */ > > +struct hv_send_ipi_ex { > > u32 vector; > > u32 reserved; > > struct hv_vpset vp_set; > >
RE: [PATCH v5 4/5] x86/hyper-v: rename ipi_arg_{ex,non_ex} structures
> -Original Message- > From: Paolo Bonzini > Sent: Friday, September 14, 2018 10:37 AM > To: vkuznets ; k...@vger.kernel.org > Cc: Radim Krčmář ; Roman Kagan > ; KY Srinivasan ; Haiyang > Zhang ; Stephen Hemminger > ; Michael Kelley (EOSG) > ; Mohammed Gamal > ; Cathy Avery ; Wanpeng Li > ; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v5 4/5] x86/hyper-v: rename ipi_arg_{ex,non_ex} > structures > > On 27/08/2018 18:48, Vitaly Kuznetsov wrote: > > These structures are going to be used from KVM code so let's make > > their names reflect their Hyper-V origin. > > > > Signed-off-by: Vitaly Kuznetsov > > Reviewed-by: Roman Kagan > > KY, can you ack this patch? Acked-by: K. Y. Srinivasan > > Thanks, > > Paolo > > > --- > > arch/x86/hyperv/hv_apic.c | 8 > > arch/x86/include/asm/hyperv-tlfs.h | 16 +--- > > 2 files changed, 13 insertions(+), 11 deletions(-) > > > > diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c > > index 5b0f613428c2..2c43e3055948 100644 > > --- a/arch/x86/hyperv/hv_apic.c > > +++ b/arch/x86/hyperv/hv_apic.c > > @@ -95,8 +95,8 @@ static void hv_apic_eoi_write(u32 reg, u32 val) > > */ > > static bool __send_ipi_mask_ex(const struct cpumask *mask, int vector) > > { > > - struct ipi_arg_ex **arg; > > - struct ipi_arg_ex *ipi_arg; > > + struct hv_send_ipi_ex **arg; > > + struct hv_send_ipi_ex *ipi_arg; > > unsigned long flags; > > int nr_bank = 0; > > int ret = 1; > > @@ -105,7 +105,7 @@ static bool __send_ipi_mask_ex(const struct > cpumask *mask, int vector) > > return false; > > > > local_irq_save(flags); > > - arg = (struct ipi_arg_ex **)this_cpu_ptr(hyperv_pcpu_input_arg); > > + arg = (struct hv_send_ipi_ex > **)this_cpu_ptr(hyperv_pcpu_input_arg); > > > > ipi_arg = *arg; > > if (unlikely(!ipi_arg)) > > @@ -135,7 +135,7 @@ static bool __send_ipi_mask_ex(const struct > cpumask *mask, int vector) > > static bool __send_ipi_mask(const struct cpumask *mask, int vector) > > { > > int cur_cpu, vcpu; > > - struct ipi_arg_non_ex ipi_arg; > > + struct hv_send_ipi ipi_arg; > > int ret = 1; > > > > trace_hyperv_send_ipi_mask(mask, vector); > > diff --git a/arch/x86/include/asm/hyperv-tlfs.h > b/arch/x86/include/asm/hyperv-tlfs.h > > index e977b6b3a538..00e01d215f74 100644 > > --- a/arch/x86/include/asm/hyperv-tlfs.h > > +++ b/arch/x86/include/asm/hyperv-tlfs.h > > @@ -726,19 +726,21 @@ struct hv_enlightened_vmcs { > > #define HV_STIMER_AUTOENABLE (1ULL << 3) > > #define HV_STIMER_SINT(config) (__u8)(((config) >> 16) & > 0x0F) > > > > -struct ipi_arg_non_ex { > > - u32 vector; > > - u32 reserved; > > - u64 cpu_mask; > > -}; > > - > > struct hv_vpset { > > u64 format; > > u64 valid_bank_mask; > > u64 bank_contents[]; > > }; > > > > -struct ipi_arg_ex { > > +/* HvCallSendSyntheticClusterIpi hypercall */ > > +struct hv_send_ipi { > > + u32 vector; > > + u32 reserved; > > + u64 cpu_mask; > > +}; > > + > > +/* HvCallSendSyntheticClusterIpiEx hypercall */ > > +struct hv_send_ipi_ex { > > u32 vector; > > u32 reserved; > > struct hv_vpset vp_set; > >
RE: [PATCH] hyper-v: Fix wakeup from suspend-to-idle
> -Original Message- > From: Rafael J. Wysocki > Sent: Wednesday, September 12, 2018 11:55 PM > To: vkuznets > Cc: Linux PM ; Rafael J. Wysocki > ; KY Srinivasan ; Haiyang Zhang > ; Stephen Hemminger > ; Jiri Kosina ; Dmitry > Torokhov ; linux-in...@vger.kernel.org; > Linux Kernel Mailing List > Subject: Re: [PATCH] hyper-v: Fix wakeup from suspend-to-idle > > On Wed, Sep 12, 2018 at 6:11 PM Vitaly Kuznetsov > wrote: > > > > It makes little sense but still possible to put Hyper-V guests into > > suspend-to-idle state. To wake them up two wakeup sources were > registered > > in the past: hyperv-keyboard and hid-hyperv. However, since > > commit eed4d47efe95 ("ACPI / sleep: Ignore spurious SCI wakeups from > > suspend-to-idle") pm_wakeup_event() from these devices is ignored. > Switch > > to pm_wakeup_hard_event() API as these devices are actually the only > > possible way to wakeup Hyper-V guests. > > > > Fixes: eed4d47efe95 (ACPI / sleep: Ignore spurious SCI wakeups from > suspend-to-idle) > > Signed-off-by: Vitaly Kuznetsov > > Reviewed-by: Rafael J. Wysocki Acked-by: K. Y. Srinivasan > > > --- > > drivers/hid/hid-hyperv.c | 2 +- > > drivers/input/serio/hyperv-keyboard.c | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c > > index b372854cf38d..704049e62d58 100644 > > --- a/drivers/hid/hid-hyperv.c > > +++ b/drivers/hid/hid-hyperv.c > > @@ -309,7 +309,7 @@ static void mousevsc_on_receive(struct hv_device > *device, > > hid_input_report(input_dev->hid_device, HID_INPUT_REPORT, > > input_dev->input_buf, len, 1); > > > > - pm_wakeup_event(_dev->device->device, 0); > > + pm_wakeup_hard_event(_dev->device->device); > > > > break; > > default: > > diff --git a/drivers/input/serio/hyperv-keyboard.c > b/drivers/input/serio/hyperv-keyboard.c > > index 47a0e81a2989..a8b9be3e28db 100644 > > --- a/drivers/input/serio/hyperv-keyboard.c > > +++ b/drivers/input/serio/hyperv-keyboard.c > > @@ -177,7 +177,7 @@ static void hv_kbd_on_receive(struct hv_device > *hv_dev, > > * state because the Enter-UP can trigger a wakeup at once. > > */ > > if (!(info & IS_BREAK)) > > - pm_wakeup_event(_dev->device, 0); > > + pm_wakeup_hard_event(_dev->device); > > > > break; > > > > -- > > 2.14.4 > >
RE: [PATCH] hyper-v: Fix wakeup from suspend-to-idle
> -Original Message- > From: Rafael J. Wysocki > Sent: Wednesday, September 12, 2018 11:55 PM > To: vkuznets > Cc: Linux PM ; Rafael J. Wysocki > ; KY Srinivasan ; Haiyang Zhang > ; Stephen Hemminger > ; Jiri Kosina ; Dmitry > Torokhov ; linux-in...@vger.kernel.org; > Linux Kernel Mailing List > Subject: Re: [PATCH] hyper-v: Fix wakeup from suspend-to-idle > > On Wed, Sep 12, 2018 at 6:11 PM Vitaly Kuznetsov > wrote: > > > > It makes little sense but still possible to put Hyper-V guests into > > suspend-to-idle state. To wake them up two wakeup sources were > registered > > in the past: hyperv-keyboard and hid-hyperv. However, since > > commit eed4d47efe95 ("ACPI / sleep: Ignore spurious SCI wakeups from > > suspend-to-idle") pm_wakeup_event() from these devices is ignored. > Switch > > to pm_wakeup_hard_event() API as these devices are actually the only > > possible way to wakeup Hyper-V guests. > > > > Fixes: eed4d47efe95 (ACPI / sleep: Ignore spurious SCI wakeups from > suspend-to-idle) > > Signed-off-by: Vitaly Kuznetsov > > Reviewed-by: Rafael J. Wysocki Acked-by: K. Y. Srinivasan > > > --- > > drivers/hid/hid-hyperv.c | 2 +- > > drivers/input/serio/hyperv-keyboard.c | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c > > index b372854cf38d..704049e62d58 100644 > > --- a/drivers/hid/hid-hyperv.c > > +++ b/drivers/hid/hid-hyperv.c > > @@ -309,7 +309,7 @@ static void mousevsc_on_receive(struct hv_device > *device, > > hid_input_report(input_dev->hid_device, HID_INPUT_REPORT, > > input_dev->input_buf, len, 1); > > > > - pm_wakeup_event(_dev->device->device, 0); > > + pm_wakeup_hard_event(_dev->device->device); > > > > break; > > default: > > diff --git a/drivers/input/serio/hyperv-keyboard.c > b/drivers/input/serio/hyperv-keyboard.c > > index 47a0e81a2989..a8b9be3e28db 100644 > > --- a/drivers/input/serio/hyperv-keyboard.c > > +++ b/drivers/input/serio/hyperv-keyboard.c > > @@ -177,7 +177,7 @@ static void hv_kbd_on_receive(struct hv_device > *hv_dev, > > * state because the Enter-UP can trigger a wakeup at once. > > */ > > if (!(info & IS_BREAK)) > > - pm_wakeup_event(_dev->device, 0); > > + pm_wakeup_hard_event(_dev->device); > > > > break; > > > > -- > > 2.14.4 > >
RE: [PATCH] Drivers: hv: vmbus: include header for get_irq_regs()
> -Original Message- > From: Sebastian Andrzej Siewior > Sent: Monday, September 10, 2018 7:07 AM > To: KY Srinivasan ; Haiyang Zhang > ; Stephen Hemminger > > Cc: de...@linuxdriverproject.org; linux-kernel@vger.kernel.org; Steven > Rostedt ; Bernhard Landauer > ; Ralf Ramsauer regensburg.de> > Subject: Re: [PATCH] Drivers: hv: vmbus: include header for get_irq_regs() > > On 2018-08-30 09:55:03 [+0200], To K. Y. Srinivasan wrote: > > On !RT the header file get_irq_regs() gets pulled in via other header files. > On > > RT it does not and the build fails: > > > > drivers/hv/vmbus_drv.c:975 implicit declaration of function > ‘get_irq_regs’ [-Werror=implicit-function-declaration] > > drivers/hv/hv.c:115 implicit declaration of function ‘get_irq_regs’ [- > Werror=implicit-function-declaration] > > > > Add the header file for get_irq_regs() in a common header so it used by > > vmbus_drv.c by hv.c for their get_irq_regs() usage. > > > > Reported-by: Bernhard Landauer > > Reported-by: Ralf Ramsauer > > Signed-off-by: Sebastian Andrzej Siewior > > ping It is in my queue; will submit it soon. K. Y > > > --- > > drivers/hv/hyperv_vmbus.h | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h > > index 72eaba3d50fc2..797f07918197c 100644 > > --- a/drivers/hv/hyperv_vmbus.h > > +++ b/drivers/hv/hyperv_vmbus.h > > @@ -31,6 +31,7 @@ > > #include > > #include > > #include > > +#include > > > > #include "hv_trace.h" > > > > -- > > 2.18.0 > >
RE: [PATCH] Drivers: hv: vmbus: include header for get_irq_regs()
> -Original Message- > From: Sebastian Andrzej Siewior > Sent: Monday, September 10, 2018 7:07 AM > To: KY Srinivasan ; Haiyang Zhang > ; Stephen Hemminger > > Cc: de...@linuxdriverproject.org; linux-kernel@vger.kernel.org; Steven > Rostedt ; Bernhard Landauer > ; Ralf Ramsauer regensburg.de> > Subject: Re: [PATCH] Drivers: hv: vmbus: include header for get_irq_regs() > > On 2018-08-30 09:55:03 [+0200], To K. Y. Srinivasan wrote: > > On !RT the header file get_irq_regs() gets pulled in via other header files. > On > > RT it does not and the build fails: > > > > drivers/hv/vmbus_drv.c:975 implicit declaration of function > ‘get_irq_regs’ [-Werror=implicit-function-declaration] > > drivers/hv/hv.c:115 implicit declaration of function ‘get_irq_regs’ [- > Werror=implicit-function-declaration] > > > > Add the header file for get_irq_regs() in a common header so it used by > > vmbus_drv.c by hv.c for their get_irq_regs() usage. > > > > Reported-by: Bernhard Landauer > > Reported-by: Ralf Ramsauer > > Signed-off-by: Sebastian Andrzej Siewior > > ping It is in my queue; will submit it soon. K. Y > > > --- > > drivers/hv/hyperv_vmbus.h | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h > > index 72eaba3d50fc2..797f07918197c 100644 > > --- a/drivers/hv/hyperv_vmbus.h > > +++ b/drivers/hv/hyperv_vmbus.h > > @@ -31,6 +31,7 @@ > > #include > > #include > > #include > > +#include > > > > #include "hv_trace.h" > > > > -- > > 2.18.0 > >
RE: [PATCH v2 2/4] arm64: hyperv: Add support for Hyper-V as a hypervisor
> -Original Message- > From: mhkelle...@gmail.com > Sent: Wednesday, August 29, 2018 10:22 AM > To: will.dea...@arm.com; catalin.mari...@arm.com; > mark.rutl...@arm.com; marc.zyng...@arm.com; linux-arm- > ker...@lists.infradead.org; gre...@linuxfoundation.org; linux- > ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de; > a...@canonical.com; vkuznets ; > jasow...@redhat.com; marcelo.ce...@canonical.com; Stephen Hemminger > ; KY Srinivasan > Cc: Michael Kelley (EOSG) > Subject: [PATCH v2 2/4] arm64: hyperv: Add support for Hyper-V as a > hypervisor > > From: Michael Kelley > > Add ARM64-specific code to enable Hyper-V. This code includes: > * Detecting Hyper-V and initializing the guest/Hyper-V interface > * Setting up Hyper-V's synthetic clocks > * Making hypercalls using the HVC instruction > * Setting up VMbus and stimer0 interrupts > * Setting up kexec and crash handlers > This code is architecture dependent code and is mostly driven by > architecture independent code in the VMbus driver in drivers/hv/hv.c > and drivers/hv/vmbus_drv.c. > > This code is built only when CONFIG_HYPERV is enabled. > > Signed-off-by: Michael Kelley > Reviewed-by: James Morris > --- > MAINTAINERS | 1 + > arch/arm64/Makefile | 1 + > arch/arm64/hyperv/Makefile | 2 + > arch/arm64/hyperv/hv_hvc.S | 54 ++ > arch/arm64/hyperv/hv_init.c | 437 > +++ > arch/arm64/hyperv/mshyperv.c | 178 ++ > 6 files changed, 673 insertions(+) > create mode 100644 arch/arm64/hyperv/Makefile > create mode 100644 arch/arm64/hyperv/hv_hvc.S > create mode 100644 arch/arm64/hyperv/hv_init.c > create mode 100644 arch/arm64/hyperv/mshyperv.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index c8db9be..3adf83c 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6777,6 +6777,7 @@ F: arch/x86/kernel/cpu/mshyperv.c > F: arch/x86/hyperv > F: arch/arm64/include/asm/hyperv-tlfs.h > F: arch/arm64/include/asm/mshyperv.h > +F: arch/arm64/hyperv > F: drivers/hid/hid-hyperv.c > F: drivers/hv/ > F: drivers/input/serio/hyperv-keyboard.c > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile > index 106039d..af1bcfc 100644 > --- a/arch/arm64/Makefile > +++ b/arch/arm64/Makefile > @@ -106,6 +106,7 @@ core-y+= arch/arm64/kernel/ > arch/arm64/mm/ > core-$(CONFIG_NET) += arch/arm64/net/ > core-$(CONFIG_KVM) += arch/arm64/kvm/ > core-$(CONFIG_XEN) += arch/arm64/xen/ > +core-$(CONFIG_HYPERV) += arch/arm64/hyperv/ > core-$(CONFIG_CRYPTO) += arch/arm64/crypto/ > libs-y := arch/arm64/lib/ $(libs-y) > core-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a > diff --git a/arch/arm64/hyperv/Makefile b/arch/arm64/hyperv/Makefile > new file mode 100644 > index 000..988eda5 > --- /dev/null > +++ b/arch/arm64/hyperv/Makefile > @@ -0,0 +1,2 @@ > +# SPDX-License-Identifier: GPL-2.0 > +obj-y:= hv_init.o hv_hvc.o mshyperv.o > diff --git a/arch/arm64/hyperv/hv_hvc.S b/arch/arm64/hyperv/hv_hvc.S > new file mode 100644 > index 000..8263696 > --- /dev/null > +++ b/arch/arm64/hyperv/hv_hvc.S > @@ -0,0 +1,54 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > + > +/* > + * Microsoft Hyper-V hypervisor invocation routines > + * > + * Copyright (C) 2018, Microsoft, Inc. > + * > + * Author : Michael Kelley > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License version 2 as > published > + * by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD > TITLE or > + * NON INFRINGEMENT. See the GNU General Public License for more > + * details. > + */ > + > +#include > + > + .text > +/* > + * Do the HVC instruction. For Hyper-V the argument is always 1. > + * x0 contains the hypercall control value, while additional registers > + * vary depending on the hypercall, and whether the hypercall arguments > + * are in memory or in registers (a "fast" hypercall per the Hyper-V > + * TLFS). When the arguments are in memory x1 is the guest physical > + * address of the input arguments, and x2 is the guest physical > + * address of the output arguments. When the arguments are in > + * registers, the register values depends on the hypercall. Note > + * that this version cannot return any values in registers. > + */ > +ENTRY(hv_do_hvc) &g
RE: [PATCH v2 2/4] arm64: hyperv: Add support for Hyper-V as a hypervisor
> -Original Message- > From: mhkelle...@gmail.com > Sent: Wednesday, August 29, 2018 10:22 AM > To: will.dea...@arm.com; catalin.mari...@arm.com; > mark.rutl...@arm.com; marc.zyng...@arm.com; linux-arm- > ker...@lists.infradead.org; gre...@linuxfoundation.org; linux- > ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de; > a...@canonical.com; vkuznets ; > jasow...@redhat.com; marcelo.ce...@canonical.com; Stephen Hemminger > ; KY Srinivasan > Cc: Michael Kelley (EOSG) > Subject: [PATCH v2 2/4] arm64: hyperv: Add support for Hyper-V as a > hypervisor > > From: Michael Kelley > > Add ARM64-specific code to enable Hyper-V. This code includes: > * Detecting Hyper-V and initializing the guest/Hyper-V interface > * Setting up Hyper-V's synthetic clocks > * Making hypercalls using the HVC instruction > * Setting up VMbus and stimer0 interrupts > * Setting up kexec and crash handlers > This code is architecture dependent code and is mostly driven by > architecture independent code in the VMbus driver in drivers/hv/hv.c > and drivers/hv/vmbus_drv.c. > > This code is built only when CONFIG_HYPERV is enabled. > > Signed-off-by: Michael Kelley > Reviewed-by: James Morris > --- > MAINTAINERS | 1 + > arch/arm64/Makefile | 1 + > arch/arm64/hyperv/Makefile | 2 + > arch/arm64/hyperv/hv_hvc.S | 54 ++ > arch/arm64/hyperv/hv_init.c | 437 > +++ > arch/arm64/hyperv/mshyperv.c | 178 ++ > 6 files changed, 673 insertions(+) > create mode 100644 arch/arm64/hyperv/Makefile > create mode 100644 arch/arm64/hyperv/hv_hvc.S > create mode 100644 arch/arm64/hyperv/hv_init.c > create mode 100644 arch/arm64/hyperv/mshyperv.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index c8db9be..3adf83c 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6777,6 +6777,7 @@ F: arch/x86/kernel/cpu/mshyperv.c > F: arch/x86/hyperv > F: arch/arm64/include/asm/hyperv-tlfs.h > F: arch/arm64/include/asm/mshyperv.h > +F: arch/arm64/hyperv > F: drivers/hid/hid-hyperv.c > F: drivers/hv/ > F: drivers/input/serio/hyperv-keyboard.c > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile > index 106039d..af1bcfc 100644 > --- a/arch/arm64/Makefile > +++ b/arch/arm64/Makefile > @@ -106,6 +106,7 @@ core-y+= arch/arm64/kernel/ > arch/arm64/mm/ > core-$(CONFIG_NET) += arch/arm64/net/ > core-$(CONFIG_KVM) += arch/arm64/kvm/ > core-$(CONFIG_XEN) += arch/arm64/xen/ > +core-$(CONFIG_HYPERV) += arch/arm64/hyperv/ > core-$(CONFIG_CRYPTO) += arch/arm64/crypto/ > libs-y := arch/arm64/lib/ $(libs-y) > core-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a > diff --git a/arch/arm64/hyperv/Makefile b/arch/arm64/hyperv/Makefile > new file mode 100644 > index 000..988eda5 > --- /dev/null > +++ b/arch/arm64/hyperv/Makefile > @@ -0,0 +1,2 @@ > +# SPDX-License-Identifier: GPL-2.0 > +obj-y:= hv_init.o hv_hvc.o mshyperv.o > diff --git a/arch/arm64/hyperv/hv_hvc.S b/arch/arm64/hyperv/hv_hvc.S > new file mode 100644 > index 000..8263696 > --- /dev/null > +++ b/arch/arm64/hyperv/hv_hvc.S > @@ -0,0 +1,54 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > + > +/* > + * Microsoft Hyper-V hypervisor invocation routines > + * > + * Copyright (C) 2018, Microsoft, Inc. > + * > + * Author : Michael Kelley > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License version 2 as > published > + * by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD > TITLE or > + * NON INFRINGEMENT. See the GNU General Public License for more > + * details. > + */ > + > +#include > + > + .text > +/* > + * Do the HVC instruction. For Hyper-V the argument is always 1. > + * x0 contains the hypercall control value, while additional registers > + * vary depending on the hypercall, and whether the hypercall arguments > + * are in memory or in registers (a "fast" hypercall per the Hyper-V > + * TLFS). When the arguments are in memory x1 is the guest physical > + * address of the input arguments, and x2 is the guest physical > + * address of the output arguments. When the arguments are in > + * registers, the register values depends on the hypercall. Note > + * that this version cannot return any values in registers. > + */ > +ENTRY(hv_do_hvc) &g
RE: [PATCH v2 1/4] arm64: hyperv: Add core Hyper-V include files
> -Original Message- > From: mhkelle...@gmail.com > Sent: Wednesday, August 29, 2018 10:22 AM > To: will.dea...@arm.com; catalin.mari...@arm.com; > mark.rutl...@arm.com; marc.zyng...@arm.com; linux-arm- > ker...@lists.infradead.org; gre...@linuxfoundation.org; linux- > ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de; > a...@canonical.com; vkuznets ; > jasow...@redhat.com; marcelo.ce...@canonical.com; Stephen Hemminger > ; KY Srinivasan > Cc: Michael Kelley (EOSG) > Subject: [PATCH v2 1/4] arm64: hyperv: Add core Hyper-V include files > > From: Michael Kelley > > hyperv-tlfs.h defines Hyper-V interfaces from the Hyper-V Top Level > Functional Spec (TLFS). The TLFS is distinctly oriented to x86/x64, > and Hyper-V has not separated out the architecture-dependent parts into > x86/x64 vs. ARM64. So hyperv-tlfs.h includes information for ARM64 > that is not yet formally published. The TLFS is available here: > > docs.microsoft.com/en-us/virtualization/hyper-v-on- > windows/reference/tlfs > > mshyperv.h defines Linux-specific structures and routines for > interacting with Hyper-V on ARM64. > > Signed-off-by: Michael Kelley > Reviewed-by: James Morris > --- > MAINTAINERS | 2 + > arch/arm64/include/asm/hyperv-tlfs.h | 338 > +++ > arch/arm64/include/asm/mshyperv.h| 295 > ++ > 3 files changed, 635 insertions(+) > create mode 100644 arch/arm64/include/asm/hyperv-tlfs.h > create mode 100644 arch/arm64/include/asm/mshyperv.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 8bef28b..c8db9be 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6775,6 +6775,8 @@ F: arch/x86/include/asm/trace/hyperv.h > F: arch/x86/include/asm/hyperv-tlfs.h > F: arch/x86/kernel/cpu/mshyperv.c > F: arch/x86/hyperv > +F: arch/arm64/include/asm/hyperv-tlfs.h > +F: arch/arm64/include/asm/mshyperv.h > F: drivers/hid/hid-hyperv.c > F: drivers/hv/ > F: drivers/input/serio/hyperv-keyboard.c > diff --git a/arch/arm64/include/asm/hyperv-tlfs.h > b/arch/arm64/include/asm/hyperv-tlfs.h > new file mode 100644 > index 000..6f46829 > --- /dev/null > +++ b/arch/arm64/include/asm/hyperv-tlfs.h > @@ -0,0 +1,338 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > + > +/* > + * This file contains definitions from the Hyper-V Hypervisor Top-Level > + * Functional Specification (TLFS): > + * > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs. > microsoft.com%2Fen-us%2Fvirtualization%2Fhyper-v-on- > windows%2Freference%2Ftlfsdata=02%7C01%7Ckys%40microsoft.co > m%7C97234dd1d1ca4ea5479f08d60dc62f1f%7C72f988bf86f141af91ab2d7cd01 > 1db47%7C1%7C0%7C636711542195781827sdata=JOwMHsJSmkwuflaJH > qgFGHa6Wd1E7k608YK4P6KY5Xs%3Dreserved=0 > + * > + * Copyright (C) 2018, Microsoft, Inc. > + * > + * Author : Michael Kelley > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License version 2 as > published > + * by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD > TITLE or > + * NON INFRINGEMENT. See the GNU General Public License for more > + * details. > + */ > + A lot of TLFS definitions are ISA independent and we are duplicating these definitions both for X86_64 and ARM_64. Perhaps we should look at splitting this file into a common and ISA specific header file. > +#ifndef _ASM_ARM64_HYPERV_H > +#define _ASM_ARM64_HYPERV_H > + > +#include > + > +/* > + * These Hyper-V registers provide information equivalent to the CPUID > + * instruction on x86/x64. > + */ > +#define HvRegisterHypervisorVersion 0x0100 /*CPUID > 0x4002 */ > +#define HvRegisterPrivilegesAndFeaturesInfo 0x0200 /*CPUID > 0x4003 */ > +#define HvRegisterFeaturesInfo 0x0201 > /*CPUID 0x4004 */ > +#define HvRegisterImplementationLimitsInfo 0x0202 /*CPUID > 0x4005 */ > +#define HvARM64RegisterInterfaceVersion 0x00090006 /*CPUID > 0x4001 */ Can we avoid the mixed case names. > + > +/* > + * Feature identification. HvRegisterPrivilegesAndFeaturesInfo returns a > + * 128-bit value with flags indicating which features are available to the > + * partition based upon the current partition privileges. The 128-bit > + * value is broken up with different portions stored in different 32-bit > + * fields in th
RE: [PATCH v2 1/4] arm64: hyperv: Add core Hyper-V include files
> -Original Message- > From: mhkelle...@gmail.com > Sent: Wednesday, August 29, 2018 10:22 AM > To: will.dea...@arm.com; catalin.mari...@arm.com; > mark.rutl...@arm.com; marc.zyng...@arm.com; linux-arm- > ker...@lists.infradead.org; gre...@linuxfoundation.org; linux- > ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de; > a...@canonical.com; vkuznets ; > jasow...@redhat.com; marcelo.ce...@canonical.com; Stephen Hemminger > ; KY Srinivasan > Cc: Michael Kelley (EOSG) > Subject: [PATCH v2 1/4] arm64: hyperv: Add core Hyper-V include files > > From: Michael Kelley > > hyperv-tlfs.h defines Hyper-V interfaces from the Hyper-V Top Level > Functional Spec (TLFS). The TLFS is distinctly oriented to x86/x64, > and Hyper-V has not separated out the architecture-dependent parts into > x86/x64 vs. ARM64. So hyperv-tlfs.h includes information for ARM64 > that is not yet formally published. The TLFS is available here: > > docs.microsoft.com/en-us/virtualization/hyper-v-on- > windows/reference/tlfs > > mshyperv.h defines Linux-specific structures and routines for > interacting with Hyper-V on ARM64. > > Signed-off-by: Michael Kelley > Reviewed-by: James Morris > --- > MAINTAINERS | 2 + > arch/arm64/include/asm/hyperv-tlfs.h | 338 > +++ > arch/arm64/include/asm/mshyperv.h| 295 > ++ > 3 files changed, 635 insertions(+) > create mode 100644 arch/arm64/include/asm/hyperv-tlfs.h > create mode 100644 arch/arm64/include/asm/mshyperv.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 8bef28b..c8db9be 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6775,6 +6775,8 @@ F: arch/x86/include/asm/trace/hyperv.h > F: arch/x86/include/asm/hyperv-tlfs.h > F: arch/x86/kernel/cpu/mshyperv.c > F: arch/x86/hyperv > +F: arch/arm64/include/asm/hyperv-tlfs.h > +F: arch/arm64/include/asm/mshyperv.h > F: drivers/hid/hid-hyperv.c > F: drivers/hv/ > F: drivers/input/serio/hyperv-keyboard.c > diff --git a/arch/arm64/include/asm/hyperv-tlfs.h > b/arch/arm64/include/asm/hyperv-tlfs.h > new file mode 100644 > index 000..6f46829 > --- /dev/null > +++ b/arch/arm64/include/asm/hyperv-tlfs.h > @@ -0,0 +1,338 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > + > +/* > + * This file contains definitions from the Hyper-V Hypervisor Top-Level > + * Functional Specification (TLFS): > + * > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs. > microsoft.com%2Fen-us%2Fvirtualization%2Fhyper-v-on- > windows%2Freference%2Ftlfsdata=02%7C01%7Ckys%40microsoft.co > m%7C97234dd1d1ca4ea5479f08d60dc62f1f%7C72f988bf86f141af91ab2d7cd01 > 1db47%7C1%7C0%7C636711542195781827sdata=JOwMHsJSmkwuflaJH > qgFGHa6Wd1E7k608YK4P6KY5Xs%3Dreserved=0 > + * > + * Copyright (C) 2018, Microsoft, Inc. > + * > + * Author : Michael Kelley > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License version 2 as > published > + * by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD > TITLE or > + * NON INFRINGEMENT. See the GNU General Public License for more > + * details. > + */ > + A lot of TLFS definitions are ISA independent and we are duplicating these definitions both for X86_64 and ARM_64. Perhaps we should look at splitting this file into a common and ISA specific header file. > +#ifndef _ASM_ARM64_HYPERV_H > +#define _ASM_ARM64_HYPERV_H > + > +#include > + > +/* > + * These Hyper-V registers provide information equivalent to the CPUID > + * instruction on x86/x64. > + */ > +#define HvRegisterHypervisorVersion 0x0100 /*CPUID > 0x4002 */ > +#define HvRegisterPrivilegesAndFeaturesInfo 0x0200 /*CPUID > 0x4003 */ > +#define HvRegisterFeaturesInfo 0x0201 > /*CPUID 0x4004 */ > +#define HvRegisterImplementationLimitsInfo 0x0202 /*CPUID > 0x4005 */ > +#define HvARM64RegisterInterfaceVersion 0x00090006 /*CPUID > 0x4001 */ Can we avoid the mixed case names. > + > +/* > + * Feature identification. HvRegisterPrivilegesAndFeaturesInfo returns a > + * 128-bit value with flags indicating which features are available to the > + * partition based upon the current partition privileges. The 128-bit > + * value is broken up with different portions stored in different 32-bit > + * fields in th
RE: linux-4.18-rc8/tools/hv/hv_kvp_daemon.c:289: dead code block ?
> -Original Message- > From: David Binderman > Sent: Monday, August 6, 2018 1:08 AM > To: KY Srinivasan ; Haiyang Zhang > ; Stephen Hemminger > ; de...@linuxdriverproject.org; linux- > ker...@vger.kernel.org > Subject: linux-4.18-rc8/tools/hv/hv_kvp_daemon.c:289: dead code block ? > > > Hello there, > > linux-4.18-rc8/tools/hv/hv_kvp_daemon.c:282] -> [linux-4.18- > rc8/tools/hv/hv_kvp_daemon.c:289]: (warning) Opposite inner 'if' condition > leads to a dead code block. > > Source code is > >for (i = 0; i < num_records; i++) { > if (memcmp(key, record[i].key, key_size)) > continue; > /* > * Found a match; just move the remaining > * entries up. > */ > if (i == num_records) { Thank you! We will fix the issue. K. Y > > Regards > > David Binderman
RE: linux-4.18-rc8/tools/hv/hv_kvp_daemon.c:289: dead code block ?
> -Original Message- > From: David Binderman > Sent: Monday, August 6, 2018 1:08 AM > To: KY Srinivasan ; Haiyang Zhang > ; Stephen Hemminger > ; de...@linuxdriverproject.org; linux- > ker...@vger.kernel.org > Subject: linux-4.18-rc8/tools/hv/hv_kvp_daemon.c:289: dead code block ? > > > Hello there, > > linux-4.18-rc8/tools/hv/hv_kvp_daemon.c:282] -> [linux-4.18- > rc8/tools/hv/hv_kvp_daemon.c:289]: (warning) Opposite inner 'if' condition > leads to a dead code block. > > Source code is > >for (i = 0; i < num_records; i++) { > if (memcmp(key, record[i].key, key_size)) > continue; > /* > * Found a match; just move the remaining > * entries up. > */ > if (i == num_records) { Thank you! We will fix the issue. K. Y > > Regards > > David Binderman
RE: [PATCH char-misc 1/1] Drivers: hv: vmbus: Cleanup synic memory free path
> -Original Message- > From: Dan Carpenter > Sent: Wednesday, August 1, 2018 11:42 PM > To: Michael Kelley (EOSG) > Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > vkuzn...@redhat.com; jasow...@redhat.com; > marcelo.ce...@canonical.com; Stephen Hemminger > ; KY Srinivasan > Subject: Re: [PATCH char-misc 1/1] Drivers: hv: vmbus: Cleanup synic > memory free path > > On Wed, Aug 01, 2018 at 03:45:13PM -0700, mhkelle...@gmail.com wrote: > > From: Michael Kelley > > > > clk_evt memory is not being freed when the synic is shutdown > > or when there is an allocation error. Add the appropriate > > kfree() call, along with a comment to clarify how the memory > > gets freed after an allocation error. Make the free path > > consistent by removing checks for NULL since kfree() and > > free_page() already do the check. > > > > Signed-off-by: Michael Kelley > > Reported-by: Dan Carpenter > > --- > > drivers/hv/hv.c | 14 -- > > 1 file changed, 8 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c > > index 8d4fe0e..1fb9a6b 100644 > > --- a/drivers/hv/hv.c > > +++ b/drivers/hv/hv.c > > @@ -240,6 +240,10 @@ int hv_synic_alloc(void) > > > > return 0; > > err: > > + /* > > +* Any memory allocations that succeeded will be freed when > > +* the caller cleans up by calling hv_synic_free() > > +*/ > > return -ENOMEM; > > } > > > > @@ -252,12 +256,10 @@ void hv_synic_free(void) > > for_each_present_cpu(cpu) { > > struct hv_per_cpu_context *hv_cpu > > = per_cpu_ptr(hv_context.cpu_context, cpu); > > > > - if (hv_cpu->synic_event_page) > > - free_page((unsigned long)hv_cpu- > >synic_event_page); > > - if (hv_cpu->synic_message_page) > > - free_page((unsigned long)hv_cpu- > >synic_message_page); > > - if (hv_cpu->post_msg_page) > > - free_page((unsigned long)hv_cpu- > >post_msg_page); > > + kfree(hv_cpu->clk_evt); > > + free_page((unsigned long)hv_cpu->synic_event_page); > > + free_page((unsigned long)hv_cpu->synic_message_page); > > + free_page((unsigned long)hv_cpu->post_msg_page); > > This looks buggy. > > We can pass NULLs to free_page() so that's fine. So the error handling > assumes > that hv_cpu->clk_evt is either NULL or allocated. Here is how it is > allocated: > >189 int hv_synic_alloc(void) >190 { >191 int cpu; >192 >193 hv_context.hv_numa_map = kcalloc(nr_node_ids, sizeof(struct > cpumask), >194 GFP_KERNEL); >195 if (hv_context.hv_numa_map == NULL) { >196 pr_err("Unable to allocate NUMA map\n"); >197 goto err; >198 } >199 >200 for_each_present_cpu(cpu) { > ^^^ > We loop over each CPU. > >201 struct hv_per_cpu_context *hv_cpu >202 = per_cpu_ptr(hv_context.cpu_context, cpu); >203 >204 memset(hv_cpu, 0, sizeof(*hv_cpu)); > ^^ > We set this cpu memory to NULL. > >205 tasklet_init(_cpu->msg_dpc, >206 vmbus_on_msg_dpc, (unsigned long) > hv_cpu); >207 >208 hv_cpu->clk_evt = kzalloc(sizeof(struct > clock_event_device), >209GFP_KERNEL); >210 if (hv_cpu->clk_evt == NULL) { > ^^^ > Let's assume this fails on the first iteration through the loop. We > haven't memset the next cpu to NULL or allocated it. But we loop over > all the cpus in the error handling. Since we didn't set everything to NULL in > hv_synic_free() then it seems like this could be a double free. It's > possible I > am misreading the code, but either it's buggy or the memset() can be > removed. > > This is a very typical bug for this style of error handling where we free > things which were never allocated. Thanks Dan! We will fix this issue soon. K. Y > >211 pr_err("Unable to allocate clock event > device\n"); >212 goto err; >213 } > > regards, > dan carpenter
RE: [PATCH char-misc 1/1] Drivers: hv: vmbus: Cleanup synic memory free path
> -Original Message- > From: Dan Carpenter > Sent: Wednesday, August 1, 2018 11:42 PM > To: Michael Kelley (EOSG) > Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > vkuzn...@redhat.com; jasow...@redhat.com; > marcelo.ce...@canonical.com; Stephen Hemminger > ; KY Srinivasan > Subject: Re: [PATCH char-misc 1/1] Drivers: hv: vmbus: Cleanup synic > memory free path > > On Wed, Aug 01, 2018 at 03:45:13PM -0700, mhkelle...@gmail.com wrote: > > From: Michael Kelley > > > > clk_evt memory is not being freed when the synic is shutdown > > or when there is an allocation error. Add the appropriate > > kfree() call, along with a comment to clarify how the memory > > gets freed after an allocation error. Make the free path > > consistent by removing checks for NULL since kfree() and > > free_page() already do the check. > > > > Signed-off-by: Michael Kelley > > Reported-by: Dan Carpenter > > --- > > drivers/hv/hv.c | 14 -- > > 1 file changed, 8 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c > > index 8d4fe0e..1fb9a6b 100644 > > --- a/drivers/hv/hv.c > > +++ b/drivers/hv/hv.c > > @@ -240,6 +240,10 @@ int hv_synic_alloc(void) > > > > return 0; > > err: > > + /* > > +* Any memory allocations that succeeded will be freed when > > +* the caller cleans up by calling hv_synic_free() > > +*/ > > return -ENOMEM; > > } > > > > @@ -252,12 +256,10 @@ void hv_synic_free(void) > > for_each_present_cpu(cpu) { > > struct hv_per_cpu_context *hv_cpu > > = per_cpu_ptr(hv_context.cpu_context, cpu); > > > > - if (hv_cpu->synic_event_page) > > - free_page((unsigned long)hv_cpu- > >synic_event_page); > > - if (hv_cpu->synic_message_page) > > - free_page((unsigned long)hv_cpu- > >synic_message_page); > > - if (hv_cpu->post_msg_page) > > - free_page((unsigned long)hv_cpu- > >post_msg_page); > > + kfree(hv_cpu->clk_evt); > > + free_page((unsigned long)hv_cpu->synic_event_page); > > + free_page((unsigned long)hv_cpu->synic_message_page); > > + free_page((unsigned long)hv_cpu->post_msg_page); > > This looks buggy. > > We can pass NULLs to free_page() so that's fine. So the error handling > assumes > that hv_cpu->clk_evt is either NULL or allocated. Here is how it is > allocated: > >189 int hv_synic_alloc(void) >190 { >191 int cpu; >192 >193 hv_context.hv_numa_map = kcalloc(nr_node_ids, sizeof(struct > cpumask), >194 GFP_KERNEL); >195 if (hv_context.hv_numa_map == NULL) { >196 pr_err("Unable to allocate NUMA map\n"); >197 goto err; >198 } >199 >200 for_each_present_cpu(cpu) { > ^^^ > We loop over each CPU. > >201 struct hv_per_cpu_context *hv_cpu >202 = per_cpu_ptr(hv_context.cpu_context, cpu); >203 >204 memset(hv_cpu, 0, sizeof(*hv_cpu)); > ^^ > We set this cpu memory to NULL. > >205 tasklet_init(_cpu->msg_dpc, >206 vmbus_on_msg_dpc, (unsigned long) > hv_cpu); >207 >208 hv_cpu->clk_evt = kzalloc(sizeof(struct > clock_event_device), >209GFP_KERNEL); >210 if (hv_cpu->clk_evt == NULL) { > ^^^ > Let's assume this fails on the first iteration through the loop. We > haven't memset the next cpu to NULL or allocated it. But we loop over > all the cpus in the error handling. Since we didn't set everything to NULL in > hv_synic_free() then it seems like this could be a double free. It's > possible I > am misreading the code, but either it's buggy or the memset() can be > removed. > > This is a very typical bug for this style of error handling where we free > things which were never allocated. Thanks Dan! We will fix this issue soon. K. Y > >211 pr_err("Unable to allocate clock event > device\n"); >212 goto err; >213 } > > regards, > dan carpenter
RE: [PATCH V3 0/4] KVM/x86/hyper-V: Introduce PV guest address space mapping flush support
> -Original Message- > From: Tianyu Lan > Sent: Thursday, July 19, 2018 1:40 AM > Cc: Tianyu Lan ; de...@linuxdriverproject.org; > Haiyang Zhang ; h...@zytor.com; > k...@vger.kernel.org; KY Srinivasan ; linux- > ker...@vger.kernel.org; mi...@redhat.com; pbonz...@redhat.com; > rkrc...@redhat.com; Stephen Hemminger ; > t...@linutronix.de; x...@kernel.org; Michael Kelley (EOSG) > ; vkuzn...@redhat.com > Subject: [PATCH V3 0/4] KVM/x86/hyper-V: Introduce PV guest address > space mapping flush support > > Hyper-V provides a para-virtualization hypercall > HvFlushGuestPhysicalAddressSpace > to flush nested VM address space mapping in l1 hypervisor and it's to reduce > overhead > of flushing ept tlb among vcpus. The tradition way is to send IPIs to all > affected > vcpus and executes INVEPT on each vcpus. It will trigger several vmexits for > IPI and > INVEPT emulation. The pv hypercall can help to flush specified ept table on > all > vcpus > via one single hypercall. > > Change since v2: >- Make ept_pointers_match as tristate "check", "match" and "mismatch". >Set "check" in vmx_set_cr3(), check all ept table pointers in > hv_remote_flush_tlb() >and call hypercall when all ept pointers are same. >- Rename kvm_arch_hv_flush_remote_tlb with > kvm_arch_flush_remote_tlb and >Rename kvm_x86_ops->hv_tlb_remote_flush with kvm_x86_ops- > >tlb_remote_flush >- Fix issue that ignore updating tlbs_dirty during calling > kvm_arch_flush_remote_tlbs() >- Merge patch "KVM/VMX: Add identical ept table pointer check" and >patch "KVM/x86: Add tlb_remote_flush callback support for vmx" > > Change since v1: >- Fix compilation error for non-x86 platform. >- Use ept_pointers_match to check condition of identical ept > table pointer and get ept pointer from struct > vcpu_vmx->ept_pointer. >- Add hyperv_nested_flush_guest_mapping ftrace support > > > > Lan Tianyu (4): > X86/Hyper-V: Add flush HvFlushGuestPhysicalAddressSpace hypercall > support > X86/Hyper-V: Add hyperv_nested_flush_guest_mapping ftrace support > KVM: Add tlb remote flush callback in kvm_x86_ops. > KVM/x86: Add tlb_remote_flush callback support for vmx > > arch/x86/hyperv/Makefile| 2 +- > arch/x86/hyperv/nested.c| 67 > ++ > arch/x86/include/asm/hyperv-tlfs.h | 8 + > arch/x86/include/asm/kvm_host.h | 11 ++ > arch/x86/include/asm/mshyperv.h | 2 ++ > arch/x86/include/asm/trace/hyperv.h | 14 > arch/x86/kvm/vmx.c | 72 > - > include/linux/kvm_host.h| 7 > virt/kvm/kvm_main.c | 3 +- > 9 files changed, 183 insertions(+), 3 deletions(-) > create mode 100644 arch/x86/hyperv/nested.c Acked-by: K. Y. Srinivasan > > -- > 2.14.3
RE: [PATCH V3 0/4] KVM/x86/hyper-V: Introduce PV guest address space mapping flush support
> -Original Message- > From: Tianyu Lan > Sent: Thursday, July 19, 2018 1:40 AM > Cc: Tianyu Lan ; de...@linuxdriverproject.org; > Haiyang Zhang ; h...@zytor.com; > k...@vger.kernel.org; KY Srinivasan ; linux- > ker...@vger.kernel.org; mi...@redhat.com; pbonz...@redhat.com; > rkrc...@redhat.com; Stephen Hemminger ; > t...@linutronix.de; x...@kernel.org; Michael Kelley (EOSG) > ; vkuzn...@redhat.com > Subject: [PATCH V3 0/4] KVM/x86/hyper-V: Introduce PV guest address > space mapping flush support > > Hyper-V provides a para-virtualization hypercall > HvFlushGuestPhysicalAddressSpace > to flush nested VM address space mapping in l1 hypervisor and it's to reduce > overhead > of flushing ept tlb among vcpus. The tradition way is to send IPIs to all > affected > vcpus and executes INVEPT on each vcpus. It will trigger several vmexits for > IPI and > INVEPT emulation. The pv hypercall can help to flush specified ept table on > all > vcpus > via one single hypercall. > > Change since v2: >- Make ept_pointers_match as tristate "check", "match" and "mismatch". >Set "check" in vmx_set_cr3(), check all ept table pointers in > hv_remote_flush_tlb() >and call hypercall when all ept pointers are same. >- Rename kvm_arch_hv_flush_remote_tlb with > kvm_arch_flush_remote_tlb and >Rename kvm_x86_ops->hv_tlb_remote_flush with kvm_x86_ops- > >tlb_remote_flush >- Fix issue that ignore updating tlbs_dirty during calling > kvm_arch_flush_remote_tlbs() >- Merge patch "KVM/VMX: Add identical ept table pointer check" and >patch "KVM/x86: Add tlb_remote_flush callback support for vmx" > > Change since v1: >- Fix compilation error for non-x86 platform. >- Use ept_pointers_match to check condition of identical ept > table pointer and get ept pointer from struct > vcpu_vmx->ept_pointer. >- Add hyperv_nested_flush_guest_mapping ftrace support > > > > Lan Tianyu (4): > X86/Hyper-V: Add flush HvFlushGuestPhysicalAddressSpace hypercall > support > X86/Hyper-V: Add hyperv_nested_flush_guest_mapping ftrace support > KVM: Add tlb remote flush callback in kvm_x86_ops. > KVM/x86: Add tlb_remote_flush callback support for vmx > > arch/x86/hyperv/Makefile| 2 +- > arch/x86/hyperv/nested.c| 67 > ++ > arch/x86/include/asm/hyperv-tlfs.h | 8 + > arch/x86/include/asm/kvm_host.h | 11 ++ > arch/x86/include/asm/mshyperv.h | 2 ++ > arch/x86/include/asm/trace/hyperv.h | 14 > arch/x86/kvm/vmx.c | 72 > - > include/linux/kvm_host.h| 7 > virt/kvm/kvm_main.c | 3 +- > 9 files changed, 183 insertions(+), 3 deletions(-) > create mode 100644 arch/x86/hyperv/nested.c Acked-by: K. Y. Srinivasan > > -- > 2.14.3
RE: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI enlightenment.
> -Original Message- > On Fri, 6 Jul 2018, Thomas Gleixner wrote: > > On Fri, 6 Jul 2018, KY Srinivasan wrote: > > > > > > > > The problem is that the wreckage is in Linus tree and needs to be fixed > > > > there, i.e. via x86/urgent. > > > > > > > > Now we have the new bits queued in x86/hyperv already which collide. > So > > > > we > > > > need to merge x86/urgent into x86/hyperv after applying the fix and > mop up > > > > the merge wreckage in x86/hyperv. > > > > > > > > I'll have a look tomorrow morning unless you beat me to it. > > > > > > I can rebase this patch against the latest tip and resend (tomorrow). > > > > That doesn't help as we need to push the original fix to Linus ... > > Applied it to x86/urgent and fixed up the '-1' sloppy hack as pointed out > by Vitaly and merged x86/urgent into x86/hyperv. > > Please check both branches for correctness. Thanks Thomas. The merge looks good. Regards, K. Y
RE: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI enlightenment.
> -Original Message- > On Fri, 6 Jul 2018, Thomas Gleixner wrote: > > On Fri, 6 Jul 2018, KY Srinivasan wrote: > > > > > > > > The problem is that the wreckage is in Linus tree and needs to be fixed > > > > there, i.e. via x86/urgent. > > > > > > > > Now we have the new bits queued in x86/hyperv already which collide. > So > > > > we > > > > need to merge x86/urgent into x86/hyperv after applying the fix and > mop up > > > > the merge wreckage in x86/hyperv. > > > > > > > > I'll have a look tomorrow morning unless you beat me to it. > > > > > > I can rebase this patch against the latest tip and resend (tomorrow). > > > > That doesn't help as we need to push the original fix to Linus ... > > Applied it to x86/urgent and fixed up the '-1' sloppy hack as pointed out > by Vitaly and merged x86/urgent into x86/hyperv. > > Please check both branches for correctness. Thanks Thomas. The merge looks good. Regards, K. Y
RE: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI enlightenment.
> -Original Message- > From: Thomas Gleixner > Sent: Thursday, July 5, 2018 3:46 PM > To: Ingo Molnar > Cc: KY Srinivasan ; x...@kernel.org; > gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > jasow...@redhat.com; h...@zytor.com; Stephen Hemminger > ; Michael Kelley (EOSG) > ; vkuzn...@redhat.com > Subject: Re: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI > enlightenment. > > On Fri, 6 Jul 2018, Ingo Molnar wrote: > > * KY Srinivasan wrote: > > > I am confused. The label ipi_mask_done was introduced in this patch > > > (the patch under question fixes a circular dependency in this patch): > > > > > > commit 68bb7bfb7985df2bd15c2dc975cb68b7a901488a > > > Author: K. Y. Srinivasan > > > Date: Wed May 16 14:53:31 2018 -0700 > > > > > > X86/Hyper-V: Enable IPI enlightenments > > > > > > Hyper-V supports hypercalls to implement IPI; use them. > > > > > > Signed-off-by: K. Y. Srinivasan > > > Signed-off-by: Thomas Gleixner > > > Reviewed-by: Michael Kelley > > > > > > This patch was committed by Thomas some weeks ago and is in linux- > next. > > > This patch is also in 4.18-rc3. > > > > And then that name was changed to a different label in: > > > > 4bd06060762b: x86/hyper-v: Use cheaper HVCALL_SEND_IPI hypercall > when possible > > > > So maybe you were testing on an older kernel. Could you try the latest - > tip? > > The problem is that the wreckage is in Linus tree and needs to be fixed > there, i.e. via x86/urgent. > > Now we have the new bits queued in x86/hyperv already which collide. So > we > need to merge x86/urgent into x86/hyperv after applying the fix and mop up > the merge wreckage in x86/hyperv. > > I'll have a look tomorrow morning unless you beat me to it. I can rebase this patch against the latest tip and resend (tomorrow). K. Y > > Thanks, > > tglx
RE: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI enlightenment.
> -Original Message- > From: Thomas Gleixner > Sent: Thursday, July 5, 2018 3:46 PM > To: Ingo Molnar > Cc: KY Srinivasan ; x...@kernel.org; > gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > jasow...@redhat.com; h...@zytor.com; Stephen Hemminger > ; Michael Kelley (EOSG) > ; vkuzn...@redhat.com > Subject: Re: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI > enlightenment. > > On Fri, 6 Jul 2018, Ingo Molnar wrote: > > * KY Srinivasan wrote: > > > I am confused. The label ipi_mask_done was introduced in this patch > > > (the patch under question fixes a circular dependency in this patch): > > > > > > commit 68bb7bfb7985df2bd15c2dc975cb68b7a901488a > > > Author: K. Y. Srinivasan > > > Date: Wed May 16 14:53:31 2018 -0700 > > > > > > X86/Hyper-V: Enable IPI enlightenments > > > > > > Hyper-V supports hypercalls to implement IPI; use them. > > > > > > Signed-off-by: K. Y. Srinivasan > > > Signed-off-by: Thomas Gleixner > > > Reviewed-by: Michael Kelley > > > > > > This patch was committed by Thomas some weeks ago and is in linux- > next. > > > This patch is also in 4.18-rc3. > > > > And then that name was changed to a different label in: > > > > 4bd06060762b: x86/hyper-v: Use cheaper HVCALL_SEND_IPI hypercall > when possible > > > > So maybe you were testing on an older kernel. Could you try the latest - > tip? > > The problem is that the wreckage is in Linus tree and needs to be fixed > there, i.e. via x86/urgent. > > Now we have the new bits queued in x86/hyperv already which collide. So > we > need to merge x86/urgent into x86/hyperv after applying the fix and mop up > the merge wreckage in x86/hyperv. > > I'll have a look tomorrow morning unless you beat me to it. I can rebase this patch against the latest tip and resend (tomorrow). K. Y > > Thanks, > > tglx
RE: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI enlightenment.
> -Original Message- > From: Ingo Molnar On Behalf Of Ingo Molnar > Sent: Thursday, July 5, 2018 8:38 AM > 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; > h...@zytor.com; Stephen Hemminger ; Michael > Kelley (EOSG) ; vkuzn...@redhat.com > Subject: Re: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI > enlightenment. > > > * KY Srinivasan wrote: > > > > > > > > -Original Message- > > > From: Ingo Molnar On Behalf Of Ingo > Molnar > > > Sent: Wednesday, July 4, 2018 9:11 AM > > > 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; > > > h...@zytor.com; Stephen Hemminger ; > Michael > > > Kelley (EOSG) ; > vkuzn...@redhat.com > > > Subject: Re: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI > > > enlightenment. > > > > > > > > > * k...@linuxonhyperv.com wrote: > > > > > > > From: "K. Y. Srinivasan" > > > > > > > > The IPI hypercalls depend on being able to map the Linux notion of CPU > ID > > > > to the hypervisor's notion of the CPU ID. The array hv_vp_index[] > provides > > > > this mapping. Code for populating this array depends on the IPI > > > functionality. > > > > Break this circular dependency. > > > > > > > > Fixes: 68bb7bfb7985 ("X86/Hyper-V: Enable IPI enlightenments") > > > > > > > > Signed-off-by: K. Y. Srinivasan > > > > Tested-by: Michael Kelley > > > > --- > > > > arch/x86/hyperv/hv_apic.c | 5 + > > > > arch/x86/hyperv/hv_init.c | 5 - > > > > arch/x86/include/asm/mshyperv.h | 2 ++ > > > > 3 files changed, 11 insertions(+), 1 deletion(-) > > > > > > Ugh, this patch wasn't even build tested, on 64-bit allyes/allmodconfig: > > > > > > arch/x86/hyperv/hv_apic.c: In function ‘__send_ipi_mask’: > > > arch/x86/hyperv/hv_apic.c:171:4: error: label ‘ipi_mask_done’ used but > not > > > defined > > > scripts/Makefile.build:317: recipe for target 'arch/x86/hyperv/hv_apic.o' > > > failed > > > make[2]: *** [arch/x86/hyperv/hv_apic.o] Error 1 > > > > Sorry Ingo. I had a clean build on the linux-next tree (tag: next-20180702) > that > > I used to base this patch. What was the tree you applied the patch to? > > If you look at the error message, it won't build against *any* tree, because > there's no 'ipi_mask_done' label either in the kernel source, or introduced > by the patch. > > So whatever tree you used it on, if you build arch/x86/hyperv/hv_apic.o it > should > be broken. Ingo, I am confused. The label ipi_mask_done was introduced in this patch (the patch under question fixes a circular dependency in this patch): commit 68bb7bfb7985df2bd15c2dc975cb68b7a901488a Author: K. Y. Srinivasan Date: Wed May 16 14:53:31 2018 -0700 X86/Hyper-V: Enable IPI enlightenments Hyper-V supports hypercalls to implement IPI; use them. Signed-off-by: K. Y. Srinivasan Signed-off-by: Thomas Gleixner Reviewed-by: Michael Kelley This patch was committed by Thomas some weeks ago and is in linux-next. This patch is also in 4.18-rc3. Regards, K. Y > > Thanks, > > Ingo
RE: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI enlightenment.
> -Original Message- > From: Ingo Molnar On Behalf Of Ingo Molnar > Sent: Thursday, July 5, 2018 8:38 AM > 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; > h...@zytor.com; Stephen Hemminger ; Michael > Kelley (EOSG) ; vkuzn...@redhat.com > Subject: Re: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI > enlightenment. > > > * KY Srinivasan wrote: > > > > > > > > -Original Message- > > > From: Ingo Molnar On Behalf Of Ingo > Molnar > > > Sent: Wednesday, July 4, 2018 9:11 AM > > > 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; > > > h...@zytor.com; Stephen Hemminger ; > Michael > > > Kelley (EOSG) ; > vkuzn...@redhat.com > > > Subject: Re: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI > > > enlightenment. > > > > > > > > > * k...@linuxonhyperv.com wrote: > > > > > > > From: "K. Y. Srinivasan" > > > > > > > > The IPI hypercalls depend on being able to map the Linux notion of CPU > ID > > > > to the hypervisor's notion of the CPU ID. The array hv_vp_index[] > provides > > > > this mapping. Code for populating this array depends on the IPI > > > functionality. > > > > Break this circular dependency. > > > > > > > > Fixes: 68bb7bfb7985 ("X86/Hyper-V: Enable IPI enlightenments") > > > > > > > > Signed-off-by: K. Y. Srinivasan > > > > Tested-by: Michael Kelley > > > > --- > > > > arch/x86/hyperv/hv_apic.c | 5 + > > > > arch/x86/hyperv/hv_init.c | 5 - > > > > arch/x86/include/asm/mshyperv.h | 2 ++ > > > > 3 files changed, 11 insertions(+), 1 deletion(-) > > > > > > Ugh, this patch wasn't even build tested, on 64-bit allyes/allmodconfig: > > > > > > arch/x86/hyperv/hv_apic.c: In function ‘__send_ipi_mask’: > > > arch/x86/hyperv/hv_apic.c:171:4: error: label ‘ipi_mask_done’ used but > not > > > defined > > > scripts/Makefile.build:317: recipe for target 'arch/x86/hyperv/hv_apic.o' > > > failed > > > make[2]: *** [arch/x86/hyperv/hv_apic.o] Error 1 > > > > Sorry Ingo. I had a clean build on the linux-next tree (tag: next-20180702) > that > > I used to base this patch. What was the tree you applied the patch to? > > If you look at the error message, it won't build against *any* tree, because > there's no 'ipi_mask_done' label either in the kernel source, or introduced > by the patch. > > So whatever tree you used it on, if you build arch/x86/hyperv/hv_apic.o it > should > be broken. Ingo, I am confused. The label ipi_mask_done was introduced in this patch (the patch under question fixes a circular dependency in this patch): commit 68bb7bfb7985df2bd15c2dc975cb68b7a901488a Author: K. Y. Srinivasan Date: Wed May 16 14:53:31 2018 -0700 X86/Hyper-V: Enable IPI enlightenments Hyper-V supports hypercalls to implement IPI; use them. Signed-off-by: K. Y. Srinivasan Signed-off-by: Thomas Gleixner Reviewed-by: Michael Kelley This patch was committed by Thomas some weeks ago and is in linux-next. This patch is also in 4.18-rc3. Regards, K. Y > > Thanks, > > Ingo
RE: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI enlightenment.
> -Original Message- > From: Ingo Molnar On Behalf Of Ingo Molnar > Sent: Wednesday, July 4, 2018 9:11 AM > 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; > h...@zytor.com; Stephen Hemminger ; Michael > Kelley (EOSG) ; vkuzn...@redhat.com > Subject: Re: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI > enlightenment. > > > * k...@linuxonhyperv.com wrote: > > > From: "K. Y. Srinivasan" > > > > The IPI hypercalls depend on being able to map the Linux notion of CPU ID > > to the hypervisor's notion of the CPU ID. The array hv_vp_index[] provides > > this mapping. Code for populating this array depends on the IPI > functionality. > > Break this circular dependency. > > > > Fixes: 68bb7bfb7985 ("X86/Hyper-V: Enable IPI enlightenments") > > > > Signed-off-by: K. Y. Srinivasan > > Tested-by: Michael Kelley > > --- > > arch/x86/hyperv/hv_apic.c | 5 + > > arch/x86/hyperv/hv_init.c | 5 - > > arch/x86/include/asm/mshyperv.h | 2 ++ > > 3 files changed, 11 insertions(+), 1 deletion(-) > > Ugh, this patch wasn't even build tested, on 64-bit allyes/allmodconfig: > > arch/x86/hyperv/hv_apic.c: In function ‘__send_ipi_mask’: > arch/x86/hyperv/hv_apic.c:171:4: error: label ‘ipi_mask_done’ used but not > defined > scripts/Makefile.build:317: recipe for target 'arch/x86/hyperv/hv_apic.o' > failed > make[2]: *** [arch/x86/hyperv/hv_apic.o] Error 1 Sorry Ingo. I had a clean build on the linux-next tree (tag: next-20180702) that I used to base this patch. What was the tree you applied the patch to? Regards, K. Y > > Thanks, > > Ingo
RE: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI enlightenment.
> -Original Message- > From: Ingo Molnar On Behalf Of Ingo Molnar > Sent: Wednesday, July 4, 2018 9:11 AM > 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; > h...@zytor.com; Stephen Hemminger ; Michael > Kelley (EOSG) ; vkuzn...@redhat.com > Subject: Re: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI > enlightenment. > > > * k...@linuxonhyperv.com wrote: > > > From: "K. Y. Srinivasan" > > > > The IPI hypercalls depend on being able to map the Linux notion of CPU ID > > to the hypervisor's notion of the CPU ID. The array hv_vp_index[] provides > > this mapping. Code for populating this array depends on the IPI > functionality. > > Break this circular dependency. > > > > Fixes: 68bb7bfb7985 ("X86/Hyper-V: Enable IPI enlightenments") > > > > Signed-off-by: K. Y. Srinivasan > > Tested-by: Michael Kelley > > --- > > arch/x86/hyperv/hv_apic.c | 5 + > > arch/x86/hyperv/hv_init.c | 5 - > > arch/x86/include/asm/mshyperv.h | 2 ++ > > 3 files changed, 11 insertions(+), 1 deletion(-) > > Ugh, this patch wasn't even build tested, on 64-bit allyes/allmodconfig: > > arch/x86/hyperv/hv_apic.c: In function ‘__send_ipi_mask’: > arch/x86/hyperv/hv_apic.c:171:4: error: label ‘ipi_mask_done’ used but not > defined > scripts/Makefile.build:317: recipe for target 'arch/x86/hyperv/hv_apic.o' > failed > make[2]: *** [arch/x86/hyperv/hv_apic.o] Error 1 Sorry Ingo. I had a clean build on the linux-next tree (tag: next-20180702) that I used to base this patch. What was the tree you applied the patch to? Regards, K. Y > > Thanks, > > Ingo
RE: [PATCH 1/1] X86: Fix the circular dependency in IPI enlightenment.
> -Original Message- > From: k...@linuxonhyperv.com > Sent: Tuesday, July 3, 2018 3:37 PM > To: 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; > h...@zytor.com; Stephen Hemminger ; Michael > Kelley (EOSG) ; vkuzn...@redhat.com > Cc: KY Srinivasan > Subject: [PATCH 1/1] X86: Fix the circular dependency in IPI enlightenment. > > From: "K. Y. Srinivasan" > > The IPI hypercalls depend on being able to map the Linux notion of CPU ID > to the hypervisor's notion of the CPU ID. The array hv_vp_index[] provides > this mapping. Code for populating this array depends on the IPI functionality. > Break this circular dependency. > > Fixes: 68bb7bfb7985 ("X86/Hyper-V: Enable IPI enlightenments") Please drop this patch; I have sent the wrong patch. Regards, K. Y > > Signed-off-by: K. Y. Srinivasan > Tested-by: Michael Kelley > --- > arch/x86/hyperv/hv_apic.c | 5 + > arch/x86/hyperv/hv_init.c | 3 +++ > arch/x86/include/asm/mshyperv.h | 2 ++ > 3 files changed, 10 insertions(+) > > diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c > index f68855499391..63d7c196739f 100644 > --- a/arch/x86/hyperv/hv_apic.c > +++ b/arch/x86/hyperv/hv_apic.c > @@ -114,6 +114,8 @@ static bool __send_ipi_mask_ex(const struct cpumask > *mask, int vector) > ipi_arg->vp_set.format = HV_GENERIC_SET_SPARSE_4K; > nr_bank = cpumask_to_vpset(&(ipi_arg->vp_set), mask); > } > + if (nr_bank == -1) > + goto ipi_mask_ex_done; > if (!nr_bank) > ipi_arg->vp_set.format = HV_GENERIC_SET_ALL; > > @@ -158,6 +160,9 @@ static bool __send_ipi_mask(const struct cpumask > *mask, int vector) > > for_each_cpu(cur_cpu, mask) { > vcpu = hv_cpu_number_to_vp_number(cur_cpu); > + if (vcpu == -1) > + goto ipi_mask_done; > + > /* >* This particular version of the IPI hypercall can >* only target upto 64 CPUs. > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c > index 595e44e8abaa..762ce164d733 100644 > --- a/arch/x86/hyperv/hv_init.c > +++ b/arch/x86/hyperv/hv_init.c > @@ -293,6 +293,9 @@ void __init hyperv_init(void) > if (!hv_vp_index) > return; > > + for (i = 0; i < num_possible_cpus(); i++) > + hv_vp_index[i] = -1; > + > hv_vp_assist_page = kcalloc(num_possible_cpus(), > sizeof(*hv_vp_assist_page), GFP_KERNEL); > if (!hv_vp_assist_page) { > diff --git a/arch/x86/include/asm/mshyperv.h > b/arch/x86/include/asm/mshyperv.h > index 81e768b8d9eb..299de3dcc319 100644 > --- a/arch/x86/include/asm/mshyperv.h > +++ b/arch/x86/include/asm/mshyperv.h > @@ -285,6 +285,8 @@ static inline int cpumask_to_vpset(struct hv_vpset > *vpset, >*/ > for_each_cpu(cpu, cpus) { > vcpu = hv_cpu_number_to_vp_number(cpu); > + if (vcpu == -1) > + return -1; > vcpu_bank = vcpu / 64; > vcpu_offset = vcpu % 64; > __set_bit(vcpu_offset, (unsigned long *) > -- > 2.17.1
RE: [PATCH 1/1] X86: Fix the circular dependency in IPI enlightenment.
> -Original Message- > From: k...@linuxonhyperv.com > Sent: Tuesday, July 3, 2018 3:37 PM > To: 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; > h...@zytor.com; Stephen Hemminger ; Michael > Kelley (EOSG) ; vkuzn...@redhat.com > Cc: KY Srinivasan > Subject: [PATCH 1/1] X86: Fix the circular dependency in IPI enlightenment. > > From: "K. Y. Srinivasan" > > The IPI hypercalls depend on being able to map the Linux notion of CPU ID > to the hypervisor's notion of the CPU ID. The array hv_vp_index[] provides > this mapping. Code for populating this array depends on the IPI functionality. > Break this circular dependency. > > Fixes: 68bb7bfb7985 ("X86/Hyper-V: Enable IPI enlightenments") Please drop this patch; I have sent the wrong patch. Regards, K. Y > > Signed-off-by: K. Y. Srinivasan > Tested-by: Michael Kelley > --- > arch/x86/hyperv/hv_apic.c | 5 + > arch/x86/hyperv/hv_init.c | 3 +++ > arch/x86/include/asm/mshyperv.h | 2 ++ > 3 files changed, 10 insertions(+) > > diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c > index f68855499391..63d7c196739f 100644 > --- a/arch/x86/hyperv/hv_apic.c > +++ b/arch/x86/hyperv/hv_apic.c > @@ -114,6 +114,8 @@ static bool __send_ipi_mask_ex(const struct cpumask > *mask, int vector) > ipi_arg->vp_set.format = HV_GENERIC_SET_SPARSE_4K; > nr_bank = cpumask_to_vpset(&(ipi_arg->vp_set), mask); > } > + if (nr_bank == -1) > + goto ipi_mask_ex_done; > if (!nr_bank) > ipi_arg->vp_set.format = HV_GENERIC_SET_ALL; > > @@ -158,6 +160,9 @@ static bool __send_ipi_mask(const struct cpumask > *mask, int vector) > > for_each_cpu(cur_cpu, mask) { > vcpu = hv_cpu_number_to_vp_number(cur_cpu); > + if (vcpu == -1) > + goto ipi_mask_done; > + > /* >* This particular version of the IPI hypercall can >* only target upto 64 CPUs. > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c > index 595e44e8abaa..762ce164d733 100644 > --- a/arch/x86/hyperv/hv_init.c > +++ b/arch/x86/hyperv/hv_init.c > @@ -293,6 +293,9 @@ void __init hyperv_init(void) > if (!hv_vp_index) > return; > > + for (i = 0; i < num_possible_cpus(); i++) > + hv_vp_index[i] = -1; > + > hv_vp_assist_page = kcalloc(num_possible_cpus(), > sizeof(*hv_vp_assist_page), GFP_KERNEL); > if (!hv_vp_assist_page) { > diff --git a/arch/x86/include/asm/mshyperv.h > b/arch/x86/include/asm/mshyperv.h > index 81e768b8d9eb..299de3dcc319 100644 > --- a/arch/x86/include/asm/mshyperv.h > +++ b/arch/x86/include/asm/mshyperv.h > @@ -285,6 +285,8 @@ static inline int cpumask_to_vpset(struct hv_vpset > *vpset, >*/ > for_each_cpu(cpu, cpus) { > vcpu = hv_cpu_number_to_vp_number(cpu); > + if (vcpu == -1) > + return -1; > vcpu_bank = vcpu / 64; > vcpu_offset = vcpu % 64; > __set_bit(vcpu_offset, (unsigned long *) > -- > 2.17.1
RE: [PATCH 3/3] KVM: x86: hyperv: implement PV IPI send hypercalls
> -Original Message- > From: Vitaly Kuznetsov > Sent: Thursday, June 28, 2018 8:05 AM > To: Wanpeng Li > Cc: Radim Krcmar ; kvm ; > Paolo Bonzini ; Roman Kagan > ; KY Srinivasan ; Haiyang > Zhang ; Stephen Hemminger > ; Michael Kelley (EOSG) > ; Mohammed Gamal > ; Cathy Avery ; LKML ker...@vger.kernel.org> > Subject: Re: [PATCH 3/3] KVM: x86: hyperv: implement PV IPI send > hypercalls > > Wanpeng Li writes: > > > Hi Paolo and Radim, > > > > I have already completed the patches for linux guest/kvm/qemu w/ vCPUs > > <= 64, however, extra complication as the ex in hyperv should be > > introduced for vCPUs > 64, so do you think vCPU <=64 is enough for > > linux guest or should me introduce two hypercall as what hyperv does > > w/ ex logic? > > > > Neither Paolo nor Radim but as we already have > #define KVM_MAX_VCPUS 288 > on at least x86, supporting <= 64 vCPUs seems to be too limiting for any > new functionality. Agreed. K. Y > > -- > Vitaly
RE: [PATCH 3/3] KVM: x86: hyperv: implement PV IPI send hypercalls
> -Original Message- > From: Vitaly Kuznetsov > Sent: Thursday, June 28, 2018 8:05 AM > To: Wanpeng Li > Cc: Radim Krcmar ; kvm ; > Paolo Bonzini ; Roman Kagan > ; KY Srinivasan ; Haiyang > Zhang ; Stephen Hemminger > ; Michael Kelley (EOSG) > ; Mohammed Gamal > ; Cathy Avery ; LKML ker...@vger.kernel.org> > Subject: Re: [PATCH 3/3] KVM: x86: hyperv: implement PV IPI send > hypercalls > > Wanpeng Li writes: > > > Hi Paolo and Radim, > > > > I have already completed the patches for linux guest/kvm/qemu w/ vCPUs > > <= 64, however, extra complication as the ex in hyperv should be > > introduced for vCPUs > 64, so do you think vCPU <=64 is enough for > > linux guest or should me introduce two hypercall as what hyperv does > > w/ ex logic? > > > > Neither Paolo nor Radim but as we already have > #define KVM_MAX_VCPUS 288 > on at least x86, supporting <= 64 vCPUs seems to be too limiting for any > new functionality. Agreed. K. Y > > -- > Vitaly
RE: [PATCH] x86/hyper-v: use cheaper HVCALL_FLUSH_VIRTUAL_ADDRESS_{LIST,SPACE} hypercalls when possible
> -Original Message- > From: Vitaly Kuznetsov > Sent: Wednesday, June 20, 2018 1:24 AM > To: Michael Kelley (EOSG) > Cc: x...@kernel.org; de...@linuxdriverproject.org; linux- > ker...@vger.kernel.org; KY Srinivasan ; Haiyang Zhang > ; Stephen Hemminger > ; Thomas Gleixner ; Ingo > Molnar ; H. Peter Anvin ; Tianyu Lan > > Subject: Re: [PATCH] x86/hyper-v: use cheaper > HVCALL_FLUSH_VIRTUAL_ADDRESS_{LIST,SPACE} hypercalls when possible > > "Michael Kelley (EOSG)" writes: > > >> -Original Message- > >> From: linux-kernel-ow...@vger.kernel.org ow...@vger.kernel.org> On Behalf > >> Of Vitaly Kuznetsov > >> Sent: Friday, June 15, 2018 9:30 AM > >> To: x...@kernel.org > >> Cc: de...@linuxdriverproject.org; linux-kernel@vger.kernel.org; KY > Srinivasan > >> ; Haiyang Zhang ; > Stephen Hemminger > >> ; Thomas Gleixner ; > Ingo Molnar > >> ; H. Peter Anvin ; Tianyu Lan > >> > >> Subject: [PATCH] x86/hyper-v: use cheaper > HVCALL_FLUSH_VIRTUAL_ADDRESS_{LIST,SPACE} > >> hypercalls when possible > >> > >> While working on Hyper-V style PV TLB flush support in KVM I noticed that > >> real Windows guests use TLB flush hypercall in a somewhat smarter way: > when > >> the flush needs to be performed on a subset of first 64 vCPUs or on all > >> present vCPUs Windows avoids more expensive hypercalls which support > >> sparse CPU sets and uses their 'cheap' counterparts. This means that > >> HV_X64_EX_PROCESSOR_MASKS_RECOMMENDED name is actually a > misnomer: EX > >> hypercalls (which support sparse CPU sets) are "available", not > >> "recommended". This makes sense as they are actually harder to parse. > >> > >> Nothing stops us from being equally 'smart' in Linux too. Switch to > >> doing cheaper hypercalls whenever possible. > >> > >> Signed-off-by: Vitaly Kuznetsov > >> --- > > > > This is a good idea. We should probably do the same with the hypercalls for > sending > > IPIs -- try the simpler version first and move to the more complex _EX > version only > > if necessary. > > > > A complication: We've recently found a problem with the code for doing > IPI > > hypercalls, and the bug affects the TLB flush code as well. As secondary > CPUs > > are started, there's a window of time where the hv_vp_index entry for a > > secondary CPU is uninitialized. We are seeing IPIs happening in that > window, and > > the IPI hypercall code uses the uninitialized hv_vp_index entry. Same > thing could > > happen with the TLB flush hypercall code. I didn't actually see any > occurrences of > > the TLB case in my tracing, but we should fix it anyway in case a TLB flush > gets > > added at some point in the future. > > > > KY has a patch coming. In the patch, hv_cpu_number_to_vp_number() > > and cpumask_to_vpset() can both return U32_MAX if they encounter an > > uninitialized hv_vp_index entry, and the code needs to be able to bail out > to > > the native functions for that particular IPI or TLB flush operation. Once > > the > > initialization of secondary CPUs is complete, the uninitialized situation > won't > > happen again, and the hypercall path will always be used. > > Sure, I am surprised that we have not seen this issue in tlb flush enlightenments. K. Y > > with TLB flush we can always fall back to doing it natively (by sending > IPIs). > > > > > We'll need to coordinate on these patches. Be aware that the IPI flavor of > the > > bug is currently causing random failures when booting 4.18 RC1 on Hyper-V > VMs > > with large vCPU counts. > > Thanks for the heads up! This particular patch is just an optimization > so there's no rush, IPI fix is definitely more important. > > > > > Reviewed-by: Michael Kelley > > Thanks! > > -- > Vitaly
RE: [PATCH] x86/hyper-v: use cheaper HVCALL_FLUSH_VIRTUAL_ADDRESS_{LIST,SPACE} hypercalls when possible
> -Original Message- > From: Vitaly Kuznetsov > Sent: Wednesday, June 20, 2018 1:24 AM > To: Michael Kelley (EOSG) > Cc: x...@kernel.org; de...@linuxdriverproject.org; linux- > ker...@vger.kernel.org; KY Srinivasan ; Haiyang Zhang > ; Stephen Hemminger > ; Thomas Gleixner ; Ingo > Molnar ; H. Peter Anvin ; Tianyu Lan > > Subject: Re: [PATCH] x86/hyper-v: use cheaper > HVCALL_FLUSH_VIRTUAL_ADDRESS_{LIST,SPACE} hypercalls when possible > > "Michael Kelley (EOSG)" writes: > > >> -Original Message- > >> From: linux-kernel-ow...@vger.kernel.org ow...@vger.kernel.org> On Behalf > >> Of Vitaly Kuznetsov > >> Sent: Friday, June 15, 2018 9:30 AM > >> To: x...@kernel.org > >> Cc: de...@linuxdriverproject.org; linux-kernel@vger.kernel.org; KY > Srinivasan > >> ; Haiyang Zhang ; > Stephen Hemminger > >> ; Thomas Gleixner ; > Ingo Molnar > >> ; H. Peter Anvin ; Tianyu Lan > >> > >> Subject: [PATCH] x86/hyper-v: use cheaper > HVCALL_FLUSH_VIRTUAL_ADDRESS_{LIST,SPACE} > >> hypercalls when possible > >> > >> While working on Hyper-V style PV TLB flush support in KVM I noticed that > >> real Windows guests use TLB flush hypercall in a somewhat smarter way: > when > >> the flush needs to be performed on a subset of first 64 vCPUs or on all > >> present vCPUs Windows avoids more expensive hypercalls which support > >> sparse CPU sets and uses their 'cheap' counterparts. This means that > >> HV_X64_EX_PROCESSOR_MASKS_RECOMMENDED name is actually a > misnomer: EX > >> hypercalls (which support sparse CPU sets) are "available", not > >> "recommended". This makes sense as they are actually harder to parse. > >> > >> Nothing stops us from being equally 'smart' in Linux too. Switch to > >> doing cheaper hypercalls whenever possible. > >> > >> Signed-off-by: Vitaly Kuznetsov > >> --- > > > > This is a good idea. We should probably do the same with the hypercalls for > sending > > IPIs -- try the simpler version first and move to the more complex _EX > version only > > if necessary. > > > > A complication: We've recently found a problem with the code for doing > IPI > > hypercalls, and the bug affects the TLB flush code as well. As secondary > CPUs > > are started, there's a window of time where the hv_vp_index entry for a > > secondary CPU is uninitialized. We are seeing IPIs happening in that > window, and > > the IPI hypercall code uses the uninitialized hv_vp_index entry. Same > thing could > > happen with the TLB flush hypercall code. I didn't actually see any > occurrences of > > the TLB case in my tracing, but we should fix it anyway in case a TLB flush > gets > > added at some point in the future. > > > > KY has a patch coming. In the patch, hv_cpu_number_to_vp_number() > > and cpumask_to_vpset() can both return U32_MAX if they encounter an > > uninitialized hv_vp_index entry, and the code needs to be able to bail out > to > > the native functions for that particular IPI or TLB flush operation. Once > > the > > initialization of secondary CPUs is complete, the uninitialized situation > won't > > happen again, and the hypercall path will always be used. > > Sure, I am surprised that we have not seen this issue in tlb flush enlightenments. K. Y > > with TLB flush we can always fall back to doing it natively (by sending > IPIs). > > > > > We'll need to coordinate on these patches. Be aware that the IPI flavor of > the > > bug is currently causing random failures when booting 4.18 RC1 on Hyper-V > VMs > > with large vCPU counts. > > Thanks for the heads up! This particular patch is just an optimization > so there's no rush, IPI fix is definitely more important. > > > > > Reviewed-by: Michael Kelley > > Thanks! > > -- > Vitaly
RE: [PATCH] x86/hyper-v: use cheaper HVCALL_FLUSH_VIRTUAL_ADDRESS_{LIST,SPACE} hypercalls when possible
> -Original Message- > From: Michael Kelley (EOSG) > Sent: Tuesday, June 19, 2018 10:57 AM > To: Vitaly Kuznetsov ; x...@kernel.org > Cc: de...@linuxdriverproject.org; linux-kernel@vger.kernel.org; KY > Srinivasan ; Haiyang Zhang > ; Stephen Hemminger > ; Thomas Gleixner ; Ingo > Molnar ; H. Peter Anvin ; Tianyu Lan > > Subject: RE: [PATCH] x86/hyper-v: use cheaper > HVCALL_FLUSH_VIRTUAL_ADDRESS_{LIST,SPACE} hypercalls when possible > > > -Original Message- > > From: linux-kernel-ow...@vger.kernel.org ow...@vger.kernel.org> On Behalf > > Of Vitaly Kuznetsov > > Sent: Friday, June 15, 2018 9:30 AM > > To: x...@kernel.org > > Cc: de...@linuxdriverproject.org; linux-kernel@vger.kernel.org; KY > Srinivasan > > ; Haiyang Zhang ; > Stephen Hemminger > > ; Thomas Gleixner ; Ingo > Molnar > > ; H. Peter Anvin ; Tianyu Lan > > > > Subject: [PATCH] x86/hyper-v: use cheaper > HVCALL_FLUSH_VIRTUAL_ADDRESS_{LIST,SPACE} > > hypercalls when possible > > > > While working on Hyper-V style PV TLB flush support in KVM I noticed that > > real Windows guests use TLB flush hypercall in a somewhat smarter way: > when > > the flush needs to be performed on a subset of first 64 vCPUs or on all > > present vCPUs Windows avoids more expensive hypercalls which support > > sparse CPU sets and uses their 'cheap' counterparts. This means that > > HV_X64_EX_PROCESSOR_MASKS_RECOMMENDED name is actually a > misnomer: EX > > hypercalls (which support sparse CPU sets) are "available", not > > "recommended". This makes sense as they are actually harder to parse. > > > > Nothing stops us from being equally 'smart' in Linux too. Switch to > > doing cheaper hypercalls whenever possible. > > > > Signed-off-by: Vitaly Kuznetsov > > --- > > This is a good idea. We should probably do the same with the hypercalls for > sending > IPIs -- try the simpler version first and move to the more complex _EX > version only > if necessary. I am not sure if this would work correctly. When I was developing the IPI enlightenment, what I remember was that the guest is expected to use the API recommended by the Hypervisor. K. Y > > A complication: We've recently found a problem with the code for doing IPI > hypercalls, and the bug affects the TLB flush code as well. As secondary CPUs > are started, there's a window of time where the hv_vp_index entry for a > secondary CPU is uninitialized. We are seeing IPIs happening in that window, > and > the IPI hypercall code uses the uninitialized hv_vp_index entry. Same thing > could > happen with the TLB flush hypercall code. I didn't actually see any > occurrences of > the TLB case in my tracing, but we should fix it anyway in case a TLB flush > gets > added at some point in the future. > > KY has a patch coming. In the patch, hv_cpu_number_to_vp_number() > and cpumask_to_vpset() can both return U32_MAX if they encounter an > uninitialized hv_vp_index entry, and the code needs to be able to bail out to > the native functions for that particular IPI or TLB flush operation. Once the > initialization of secondary CPUs is complete, the uninitialized situation > won't > happen again, and the hypercall path will always be used. > > We'll need to coordinate on these patches. Be aware that the IPI flavor of > the > bug is currently causing random failures when booting 4.18 RC1 on Hyper-V > VMs > with large vCPU counts. > > Reviewed-by: Michael Kelley
RE: [PATCH] x86/hyper-v: use cheaper HVCALL_FLUSH_VIRTUAL_ADDRESS_{LIST,SPACE} hypercalls when possible
> -Original Message- > From: Michael Kelley (EOSG) > Sent: Tuesday, June 19, 2018 10:57 AM > To: Vitaly Kuznetsov ; x...@kernel.org > Cc: de...@linuxdriverproject.org; linux-kernel@vger.kernel.org; KY > Srinivasan ; Haiyang Zhang > ; Stephen Hemminger > ; Thomas Gleixner ; Ingo > Molnar ; H. Peter Anvin ; Tianyu Lan > > Subject: RE: [PATCH] x86/hyper-v: use cheaper > HVCALL_FLUSH_VIRTUAL_ADDRESS_{LIST,SPACE} hypercalls when possible > > > -Original Message- > > From: linux-kernel-ow...@vger.kernel.org ow...@vger.kernel.org> On Behalf > > Of Vitaly Kuznetsov > > Sent: Friday, June 15, 2018 9:30 AM > > To: x...@kernel.org > > Cc: de...@linuxdriverproject.org; linux-kernel@vger.kernel.org; KY > Srinivasan > > ; Haiyang Zhang ; > Stephen Hemminger > > ; Thomas Gleixner ; Ingo > Molnar > > ; H. Peter Anvin ; Tianyu Lan > > > > Subject: [PATCH] x86/hyper-v: use cheaper > HVCALL_FLUSH_VIRTUAL_ADDRESS_{LIST,SPACE} > > hypercalls when possible > > > > While working on Hyper-V style PV TLB flush support in KVM I noticed that > > real Windows guests use TLB flush hypercall in a somewhat smarter way: > when > > the flush needs to be performed on a subset of first 64 vCPUs or on all > > present vCPUs Windows avoids more expensive hypercalls which support > > sparse CPU sets and uses their 'cheap' counterparts. This means that > > HV_X64_EX_PROCESSOR_MASKS_RECOMMENDED name is actually a > misnomer: EX > > hypercalls (which support sparse CPU sets) are "available", not > > "recommended". This makes sense as they are actually harder to parse. > > > > Nothing stops us from being equally 'smart' in Linux too. Switch to > > doing cheaper hypercalls whenever possible. > > > > Signed-off-by: Vitaly Kuznetsov > > --- > > This is a good idea. We should probably do the same with the hypercalls for > sending > IPIs -- try the simpler version first and move to the more complex _EX > version only > if necessary. I am not sure if this would work correctly. When I was developing the IPI enlightenment, what I remember was that the guest is expected to use the API recommended by the Hypervisor. K. Y > > A complication: We've recently found a problem with the code for doing IPI > hypercalls, and the bug affects the TLB flush code as well. As secondary CPUs > are started, there's a window of time where the hv_vp_index entry for a > secondary CPU is uninitialized. We are seeing IPIs happening in that window, > and > the IPI hypercall code uses the uninitialized hv_vp_index entry. Same thing > could > happen with the TLB flush hypercall code. I didn't actually see any > occurrences of > the TLB case in my tracing, but we should fix it anyway in case a TLB flush > gets > added at some point in the future. > > KY has a patch coming. In the patch, hv_cpu_number_to_vp_number() > and cpumask_to_vpset() can both return U32_MAX if they encounter an > uninitialized hv_vp_index entry, and the code needs to be able to bail out to > the native functions for that particular IPI or TLB flush operation. Once the > initialization of secondary CPUs is complete, the uninitialized situation > won't > happen again, and the hypercall path will always be used. > > We'll need to coordinate on these patches. Be aware that the IPI flavor of > the > bug is currently causing random failures when booting 4.18 RC1 on Hyper-V > VMs > with large vCPU counts. > > Reviewed-by: Michael Kelley
RE: [PATCH] HV: Fix definition of struct hv_vp_assist_page.
+Allen > -Original Message- > From: Thomas Gleixner > Sent: Wednesday, June 6, 2018 3:18 AM > To: Tianyu Lan > Cc: KY Srinivasan ; Haiyang Zhang > ; Stephen Hemminger > ; mi...@redhat.com; h...@zytor.com; > x...@kernel.org; vkuzn...@redhat.com; Alexander Grest > ; de...@linuxdriverproject.org; linux- > ker...@vger.kernel.org > Subject: Re: [PATCH] HV: Fix definition of struct hv_vp_assist_page. > > On Mon, 21 May 2018, Tianyu Lan wrote: > > KY I am looking at the published Hyper-V Top Level Functional Spec now; will get back shortly. > > > The struct hv_vp_assist_page was defined incorrectly. > > The "vtl_control" should be u64[3], "nested_enlightenments_control" > > should be a u64 and there is 7 reserved bytes following > "enlighten_vmentry". > > This patch is to fix it. > > > > Signed-off-by: Lan Tianyu > > --- > > arch/x86/include/asm/hyperv-tlfs.h | 9 + > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/arch/x86/include/asm/hyperv-tlfs.h > b/arch/x86/include/asm/hyperv-tlfs.h > > index f7be6d03a310..fae0a5431cdd 100644 > > --- a/arch/x86/include/asm/hyperv-tlfs.h > > +++ b/arch/x86/include/asm/hyperv-tlfs.h > > @@ -496,10 +496,11 @@ struct hv_timer_message_payload { > > /* Define virtual processor assist page structure. */ > > struct hv_vp_assist_page { > > __u32 apic_assist; > > - __u32 reserved; > > - __u64 vtl_control[2]; > > - __u64 nested_enlightenments_control[2]; > > - __u32 enlighten_vmentry; > > + __u32 reserved1; > > + __u64 vtl_control[3]; > > + __u64 nested_enlightenments_control; > > + __u8 enlighten_vmentry; > > + __u8 reserved2[7]; > > __u64 current_nested_vmcs; > > }; > > > > -- > > 2.14.3 > >
RE: [PATCH] HV: Fix definition of struct hv_vp_assist_page.
+Allen > -Original Message- > From: Thomas Gleixner > Sent: Wednesday, June 6, 2018 3:18 AM > To: Tianyu Lan > Cc: KY Srinivasan ; Haiyang Zhang > ; Stephen Hemminger > ; mi...@redhat.com; h...@zytor.com; > x...@kernel.org; vkuzn...@redhat.com; Alexander Grest > ; de...@linuxdriverproject.org; linux- > ker...@vger.kernel.org > Subject: Re: [PATCH] HV: Fix definition of struct hv_vp_assist_page. > > On Mon, 21 May 2018, Tianyu Lan wrote: > > KY I am looking at the published Hyper-V Top Level Functional Spec now; will get back shortly. > > > The struct hv_vp_assist_page was defined incorrectly. > > The "vtl_control" should be u64[3], "nested_enlightenments_control" > > should be a u64 and there is 7 reserved bytes following > "enlighten_vmentry". > > This patch is to fix it. > > > > Signed-off-by: Lan Tianyu > > --- > > arch/x86/include/asm/hyperv-tlfs.h | 9 + > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/arch/x86/include/asm/hyperv-tlfs.h > b/arch/x86/include/asm/hyperv-tlfs.h > > index f7be6d03a310..fae0a5431cdd 100644 > > --- a/arch/x86/include/asm/hyperv-tlfs.h > > +++ b/arch/x86/include/asm/hyperv-tlfs.h > > @@ -496,10 +496,11 @@ struct hv_timer_message_payload { > > /* Define virtual processor assist page structure. */ > > struct hv_vp_assist_page { > > __u32 apic_assist; > > - __u32 reserved; > > - __u64 vtl_control[2]; > > - __u64 nested_enlightenments_control[2]; > > - __u32 enlighten_vmentry; > > + __u32 reserved1; > > + __u64 vtl_control[3]; > > + __u64 nested_enlightenments_control; > > + __u8 enlighten_vmentry; > > + __u8 reserved2[7]; > > __u64 current_nested_vmcs; > > }; > > > > -- > > 2.14.3 > >
RE: [tip:x86/hyperv] x86/Hyper-V/hv_apic: Include asm/apic.h
> -Original Message- > From: tip tree robot <tip...@zytor.com> > Sent: Saturday, May 19, 2018 7:46 AM > To: linux-tip-comm...@vger.kernel.org > Cc: h...@zytor.com; Michael Kelley (EOSG) > <michael.h.kel...@microsoft.com>; KY Srinivasan <k...@microsoft.com>; > t...@linutronix.de; mi...@kernel.org; l...@intel.com > Subject: [tip:x86/hyperv] x86/Hyper-V/hv_apic: Include asm/apic.h > > Commit-ID: 61eeb1f6d1f2648a218855d7c8d44f16df242ef3 > Gitweb: > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.ker > nel.org%2Ftip%2F61eeb1f6d1f2648a218855d7c8d44f16df242ef3=02%7 > C01%7Ckys%40microsoft.com%7C9469b1fea91344d65b9f08d5bd973c7c%7C7 > 2f988bf86f141af91ab2d7cd011db47%7C1%7C1%7C636623379628726325 > a=mbprYiPRtCsbYHGm%2BMsmUUt4MGGMFloyTN5Y40JrnKg%3D > =0 > Author: Thomas Gleixner <t...@linutronix.de> > AuthorDate: Sat, 19 May 2018 16:38:59 +0200 > Committer: Thomas Gleixner <t...@linutronix.de> > CommitDate: Sat, 19 May 2018 16:41:59 +0200 > > x86/Hyper-V/hv_apic: Include asm/apic.h > > Not all configurations magically include asm/apic.h, but the Hyper-V code > requires it. Include it explicitely. Thanks Thomas. I was planning to send this patch today after I saw the kbuild robot mail. Regards, K. Y > > Fixes: 6b48cb5f8347 ("X86/Hyper-V: Enlighten APIC access") > Reported-by: kbuild test robot <l...@intel.com> > Signed-off-by: Thomas Gleixner <t...@linutronix.de> > Cc: K. Y. Srinivasan <k...@microsoft.com> > Cc: Michael Kelley <mikel...@microsoft.com> > --- > arch/x86/hyperv/hv_apic.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c > index 192b6ad6a361..d3ff6e255924 100644 > --- a/arch/x86/hyperv/hv_apic.c > +++ b/arch/x86/hyperv/hv_apic.c > @@ -29,6 +29,7 @@ > #include > #include > #include > +#include > > #ifdef CONFIG_X86_64 > #if IS_ENABLED(CONFIG_HYPERV)
RE: [tip:x86/hyperv] x86/Hyper-V/hv_apic: Build the Hyper-V APIC conditionally
> -Original Message- > From: tip tree robot <tip...@zytor.com> > Sent: Saturday, May 19, 2018 12:40 PM > To: linux-tip-comm...@vger.kernel.org > Cc: l...@intel.com; h...@zytor.com; mi...@kernel.org; Michael Kelley > (EOSG) <michael.h.kel...@microsoft.com>; t...@linutronix.de; KY > Srinivasan <k...@microsoft.com> > Subject: [tip:x86/hyperv] x86/Hyper-V/hv_apic: Build the Hyper-V APIC > conditionally > > Commit-ID: 2d2ccf24939cf369f7473c7e4ea309891be91848 > Gitweb: > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.ker > nel.org%2Ftip%2F2d2ccf24939cf369f7473c7e4ea309891be91848=02%7C > 01%7Ckys%40microsoft.com%7C4ca32d3abd084c7a96b608d5bdc04fd9%7C72 > f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636623556042008674 > =F04rsVvpkPMwuzsQF6odQX2jER84S4AYcVQr4CB0yc0%3D=0 > Author: Thomas Gleixner <t...@linutronix.de> > AuthorDate: Sat, 19 May 2018 21:22:48 +0200 > Committer: Thomas Gleixner <t...@linutronix.de> > CommitDate: Sat, 19 May 2018 21:34:11 +0200 > > x86/Hyper-V/hv_apic: Build the Hyper-V APIC conditionally > > The Hyper-V APIC code is built when CONFIG_HYPERV is enabled but the > actual > code in that file is guarded with CONFIG_X86_64. There is no point in doing > this. Neither is there a point in having the CONFIG_HYPERV guard in there > because the containing directory is not built when CONFIG_HYPERV=n. > > Further for the hv_init_apic() function a stub is provided only for > CONFIG_HYPERV=n, which is pointless as the callsite is not compiled at > all. But for X86_32 the stub is missing and the build fails. > > Clean that up: > > - Compile hv_apic.c only when CONFIG_X86_64=y > - Make the stub for hv_init_apic() available when CONFG_X86_64=n Thanks Thomas. I was planning to send this patch today after I saw the kbuild robot mail. Regards, K. Y > > Fixes: 6b48cb5f8347 ("X86/Hyper-V: Enlighten APIC access") > Reported-by: kbuild test robot <l...@intel.com> > Signed-off-by: Thomas Gleixner <t...@linutronix.de> > Cc: K. Y. Srinivasan <k...@microsoft.com> > Cc: Michael Kelley <mikel...@microsoft.com> > --- > arch/x86/hyperv/Makefile| 3 ++- > arch/x86/hyperv/hv_apic.c | 6 -- > arch/x86/include/asm/mshyperv.h | 7 ++- > 3 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/arch/x86/hyperv/Makefile b/arch/x86/hyperv/Makefile > index 00ce4df01a09..b173d404e3df 100644 > --- a/arch/x86/hyperv/Makefile > +++ b/arch/x86/hyperv/Makefile > @@ -1 +1,2 @@ > -obj-y:= hv_init.o mmu.o hv_apic.o > +obj-y:= hv_init.o mmu.o > +obj-$(CONFIG_X86_64) += hv_apic.o > diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c > index d3ff6e255924..f68855499391 100644 > --- a/arch/x86/hyperv/hv_apic.c > +++ b/arch/x86/hyperv/hv_apic.c > @@ -31,9 +31,6 @@ > #include > #include > > -#ifdef CONFIG_X86_64 > -#if IS_ENABLED(CONFIG_HYPERV) > - > static struct apic orig_apic; > > static u64 hv_apic_icr_read(void) > @@ -257,6 +254,3 @@ void __init hv_apic_init(void) > apic->icr_read = hv_apic_icr_read; > } > } > - > -#endif /* CONFIG_HYPERV */ > -#endif /* CONFIG_X86_64 */ > diff --git a/arch/x86/include/asm/mshyperv.h > b/arch/x86/include/asm/mshyperv.h > index 9aaa493f5756..997192131b7b 100644 > --- a/arch/x86/include/asm/mshyperv.h > +++ b/arch/x86/include/asm/mshyperv.h > @@ -302,7 +302,13 @@ void hyperv_reenlightenment_intr(struct pt_regs > *regs); > void set_hv_tscchange_cb(void (*cb)(void)); > void clear_hv_tscchange_cb(void); > void hyperv_stop_tsc_emulation(void); > + > +#ifdef CONFIG_X86_64 > void hv_apic_init(void); > +#else > +static inline void hv_apic_init(void) {} > +#endif > + > #else /* CONFIG_HYPERV */ > static inline void hyperv_init(void) {} > static inline bool hv_is_hyperv_initialized(void) { return false; } > @@ -311,7 +317,6 @@ static inline void hyperv_setup_mmu_ops(void) {} > static inline void set_hv_tscchange_cb(void (*cb)(void)) {} > static inline void clear_hv_tscchange_cb(void) {} > static inline void hyperv_stop_tsc_emulation(void) {}; > -static inline void hv_apic_init(void) {} > static inline struct hv_vp_assist_page *hv_get_vp_assist_page(unsigned int > cpu) > { > return NULL;
RE: [tip:x86/hyperv] x86/Hyper-V/hv_apic: Include asm/apic.h
> -Original Message- > From: tip tree robot > Sent: Saturday, May 19, 2018 7:46 AM > To: linux-tip-comm...@vger.kernel.org > Cc: h...@zytor.com; Michael Kelley (EOSG) > ; KY Srinivasan ; > t...@linutronix.de; mi...@kernel.org; l...@intel.com > Subject: [tip:x86/hyperv] x86/Hyper-V/hv_apic: Include asm/apic.h > > Commit-ID: 61eeb1f6d1f2648a218855d7c8d44f16df242ef3 > Gitweb: > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.ker > nel.org%2Ftip%2F61eeb1f6d1f2648a218855d7c8d44f16df242ef3=02%7 > C01%7Ckys%40microsoft.com%7C9469b1fea91344d65b9f08d5bd973c7c%7C7 > 2f988bf86f141af91ab2d7cd011db47%7C1%7C1%7C636623379628726325 > a=mbprYiPRtCsbYHGm%2BMsmUUt4MGGMFloyTN5Y40JrnKg%3D > =0 > Author: Thomas Gleixner > AuthorDate: Sat, 19 May 2018 16:38:59 +0200 > Committer: Thomas Gleixner > CommitDate: Sat, 19 May 2018 16:41:59 +0200 > > x86/Hyper-V/hv_apic: Include asm/apic.h > > Not all configurations magically include asm/apic.h, but the Hyper-V code > requires it. Include it explicitely. Thanks Thomas. I was planning to send this patch today after I saw the kbuild robot mail. Regards, K. Y > > Fixes: 6b48cb5f8347 ("X86/Hyper-V: Enlighten APIC access") > Reported-by: kbuild test robot > Signed-off-by: Thomas Gleixner > Cc: K. Y. Srinivasan > Cc: Michael Kelley > --- > arch/x86/hyperv/hv_apic.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c > index 192b6ad6a361..d3ff6e255924 100644 > --- a/arch/x86/hyperv/hv_apic.c > +++ b/arch/x86/hyperv/hv_apic.c > @@ -29,6 +29,7 @@ > #include > #include > #include > +#include > > #ifdef CONFIG_X86_64 > #if IS_ENABLED(CONFIG_HYPERV)
RE: [tip:x86/hyperv] x86/Hyper-V/hv_apic: Build the Hyper-V APIC conditionally
> -Original Message- > From: tip tree robot > Sent: Saturday, May 19, 2018 12:40 PM > To: linux-tip-comm...@vger.kernel.org > Cc: l...@intel.com; h...@zytor.com; mi...@kernel.org; Michael Kelley > (EOSG) ; t...@linutronix.de; KY > Srinivasan > Subject: [tip:x86/hyperv] x86/Hyper-V/hv_apic: Build the Hyper-V APIC > conditionally > > Commit-ID: 2d2ccf24939cf369f7473c7e4ea309891be91848 > Gitweb: > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.ker > nel.org%2Ftip%2F2d2ccf24939cf369f7473c7e4ea309891be91848=02%7C > 01%7Ckys%40microsoft.com%7C4ca32d3abd084c7a96b608d5bdc04fd9%7C72 > f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636623556042008674 > =F04rsVvpkPMwuzsQF6odQX2jER84S4AYcVQr4CB0yc0%3D=0 > Author: Thomas Gleixner > AuthorDate: Sat, 19 May 2018 21:22:48 +0200 > Committer: Thomas Gleixner > CommitDate: Sat, 19 May 2018 21:34:11 +0200 > > x86/Hyper-V/hv_apic: Build the Hyper-V APIC conditionally > > The Hyper-V APIC code is built when CONFIG_HYPERV is enabled but the > actual > code in that file is guarded with CONFIG_X86_64. There is no point in doing > this. Neither is there a point in having the CONFIG_HYPERV guard in there > because the containing directory is not built when CONFIG_HYPERV=n. > > Further for the hv_init_apic() function a stub is provided only for > CONFIG_HYPERV=n, which is pointless as the callsite is not compiled at > all. But for X86_32 the stub is missing and the build fails. > > Clean that up: > > - Compile hv_apic.c only when CONFIG_X86_64=y > - Make the stub for hv_init_apic() available when CONFG_X86_64=n Thanks Thomas. I was planning to send this patch today after I saw the kbuild robot mail. Regards, K. Y > > Fixes: 6b48cb5f8347 ("X86/Hyper-V: Enlighten APIC access") > Reported-by: kbuild test robot > Signed-off-by: Thomas Gleixner > Cc: K. Y. Srinivasan > Cc: Michael Kelley > --- > arch/x86/hyperv/Makefile| 3 ++- > arch/x86/hyperv/hv_apic.c | 6 -- > arch/x86/include/asm/mshyperv.h | 7 ++- > 3 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/arch/x86/hyperv/Makefile b/arch/x86/hyperv/Makefile > index 00ce4df01a09..b173d404e3df 100644 > --- a/arch/x86/hyperv/Makefile > +++ b/arch/x86/hyperv/Makefile > @@ -1 +1,2 @@ > -obj-y:= hv_init.o mmu.o hv_apic.o > +obj-y:= hv_init.o mmu.o > +obj-$(CONFIG_X86_64) += hv_apic.o > diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c > index d3ff6e255924..f68855499391 100644 > --- a/arch/x86/hyperv/hv_apic.c > +++ b/arch/x86/hyperv/hv_apic.c > @@ -31,9 +31,6 @@ > #include > #include > > -#ifdef CONFIG_X86_64 > -#if IS_ENABLED(CONFIG_HYPERV) > - > static struct apic orig_apic; > > static u64 hv_apic_icr_read(void) > @@ -257,6 +254,3 @@ void __init hv_apic_init(void) > apic->icr_read = hv_apic_icr_read; > } > } > - > -#endif /* CONFIG_HYPERV */ > -#endif /* CONFIG_X86_64 */ > diff --git a/arch/x86/include/asm/mshyperv.h > b/arch/x86/include/asm/mshyperv.h > index 9aaa493f5756..997192131b7b 100644 > --- a/arch/x86/include/asm/mshyperv.h > +++ b/arch/x86/include/asm/mshyperv.h > @@ -302,7 +302,13 @@ void hyperv_reenlightenment_intr(struct pt_regs > *regs); > void set_hv_tscchange_cb(void (*cb)(void)); > void clear_hv_tscchange_cb(void); > void hyperv_stop_tsc_emulation(void); > + > +#ifdef CONFIG_X86_64 > void hv_apic_init(void); > +#else > +static inline void hv_apic_init(void) {} > +#endif > + > #else /* CONFIG_HYPERV */ > static inline void hyperv_init(void) {} > static inline bool hv_is_hyperv_initialized(void) { return false; } > @@ -311,7 +317,6 @@ static inline void hyperv_setup_mmu_ops(void) {} > static inline void set_hv_tscchange_cb(void (*cb)(void)) {} > static inline void clear_hv_tscchange_cb(void) {} > static inline void hyperv_stop_tsc_emulation(void) {}; > -static inline void hv_apic_init(void) {} > static inline struct hv_vp_assist_page *hv_get_vp_assist_page(unsigned int > cpu) > { > return NULL;
RE: [PATCH v4 1/8] x86/hyper-v: move struct hv_flush_pcpu{,ex} definitions to common header
> -Original Message- > From: Vitaly Kuznetsov <vkuzn...@redhat.com> > Sent: Thursday, May 17, 2018 12:06 AM > To: KY Srinivasan <k...@microsoft.com> > Cc: k...@vger.kernel.org; x...@kernel.org; Paolo Bonzini > <pbonz...@redhat.com>; Radim Krčmář <rkrc...@redhat.com>; Roman > Kagan <rka...@virtuozzo.com>; Haiyang Zhang <haiya...@microsoft.com>; > Stephen Hemminger <sthem...@microsoft.com>; Michael Kelley (EOSG) > <michael.h.kel...@microsoft.com>; Mohammed Gamal > <mmo...@redhat.com>; Cathy Avery <cav...@redhat.com>; linux- > ker...@vger.kernel.org > Subject: Re: [PATCH v4 1/8] x86/hyper-v: move struct hv_flush_pcpu{,ex} > definitions to common header > > KY Srinivasan <k...@microsoft.com> writes: > > >> -Original Message- > >> From: Vitaly Kuznetsov <vkuzn...@redhat.com> > >> Sent: Wednesday, May 16, 2018 8:21 AM > >> To: k...@vger.kernel.org > >> Cc: x...@kernel.org; Paolo Bonzini <pbonz...@redhat.com>; Radim > Krčmář > >> <rkrc...@redhat.com>; Roman Kagan <rka...@virtuozzo.com>; KY > >> Srinivasan <k...@microsoft.com>; Haiyang Zhang > >> <haiya...@microsoft.com>; Stephen Hemminger > >> <sthem...@microsoft.com>; Michael Kelley (EOSG) > >> <michael.h.kel...@microsoft.com>; Mohammed Gamal > >> <mmo...@redhat.com>; Cathy Avery <cav...@redhat.com>; linux- > >> ker...@vger.kernel.org > >> Subject: [PATCH v4 1/8] x86/hyper-v: move struct hv_flush_pcpu{,ex} > >> definitions to common header > >> > >> Hyper-V TLB flush hypercalls definitions will be required for KVM so move > >> them hyperv-tlfs.h. Structures also need to be renamed as '_pcpu' suffix > is > >> irrelevant for a general-purpose definition. > >> > >> Signed-off-by: Vitaly Kuznetsov <vkuzn...@redhat.com> > >> --- > >> arch/x86/hyperv/mmu.c | 40 > >> ++ > >> arch/x86/include/asm/hyperv-tlfs.h | 20 +++ > >> 2 files changed, 30 insertions(+), 30 deletions(-) > > > > Vitaly, > > > > We should coordinate on this; I have patches in flight that conflict with > > the changes here. > > I see you also fixed 'HV_GENERIC_SET_SPARCE_4K' typo. I don't think it's > a big deal as we fix it in the same way :-) > > I see you also altered hv_flush_pcpu_ex definition but kept it in mmu.c: > I think my patches should be applied on top of yours, I only need to > move it to hyperv-tlfs.h header and get rid of _pcpu suffix. > > I hope Thomas will merge your patches very soon. In case Paolo/Radim > decide that my series is ready too we'll ask their guidance on how to > resolve the conflict (topic branch?). It should be relatively easy. I agree; waiting for Thomas to merge. K. Y > > Thanks, > > -- > Vitaly
RE: [PATCH v4 1/8] x86/hyper-v: move struct hv_flush_pcpu{,ex} definitions to common header
> -Original Message- > From: Vitaly Kuznetsov > Sent: Thursday, May 17, 2018 12:06 AM > To: KY Srinivasan > Cc: k...@vger.kernel.org; x...@kernel.org; Paolo Bonzini > ; Radim Krčmář ; Roman > Kagan ; Haiyang Zhang ; > Stephen Hemminger ; Michael Kelley (EOSG) > ; Mohammed Gamal > ; Cathy Avery ; linux- > ker...@vger.kernel.org > Subject: Re: [PATCH v4 1/8] x86/hyper-v: move struct hv_flush_pcpu{,ex} > definitions to common header > > KY Srinivasan writes: > > >> -Original Message- > >> From: Vitaly Kuznetsov > >> Sent: Wednesday, May 16, 2018 8:21 AM > >> To: k...@vger.kernel.org > >> Cc: x...@kernel.org; Paolo Bonzini ; Radim > Krčmář > >> ; Roman Kagan ; KY > >> Srinivasan ; Haiyang Zhang > >> ; Stephen Hemminger > >> ; Michael Kelley (EOSG) > >> ; Mohammed Gamal > >> ; Cathy Avery ; linux- > >> ker...@vger.kernel.org > >> Subject: [PATCH v4 1/8] x86/hyper-v: move struct hv_flush_pcpu{,ex} > >> definitions to common header > >> > >> Hyper-V TLB flush hypercalls definitions will be required for KVM so move > >> them hyperv-tlfs.h. Structures also need to be renamed as '_pcpu' suffix > is > >> irrelevant for a general-purpose definition. > >> > >> Signed-off-by: Vitaly Kuznetsov > >> --- > >> arch/x86/hyperv/mmu.c | 40 > >> ++ > >> arch/x86/include/asm/hyperv-tlfs.h | 20 +++ > >> 2 files changed, 30 insertions(+), 30 deletions(-) > > > > Vitaly, > > > > We should coordinate on this; I have patches in flight that conflict with > > the changes here. > > I see you also fixed 'HV_GENERIC_SET_SPARCE_4K' typo. I don't think it's > a big deal as we fix it in the same way :-) > > I see you also altered hv_flush_pcpu_ex definition but kept it in mmu.c: > I think my patches should be applied on top of yours, I only need to > move it to hyperv-tlfs.h header and get rid of _pcpu suffix. > > I hope Thomas will merge your patches very soon. In case Paolo/Radim > decide that my series is ready too we'll ask their guidance on how to > resolve the conflict (topic branch?). It should be relatively easy. I agree; waiting for Thomas to merge. K. Y > > Thanks, > > -- > Vitaly
RE: [PATCH V2 0/5] X86: Hyper-V: APIC enlightenments
> -Original Message- > From: k...@linuxonhyperv.com <k...@linuxonhyperv.com> > Sent: Thursday, May 3, 2018 11:07 PM > To: 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; > h...@zytor.com; Stephen Hemminger <sthem...@microsoft.com>; Michael > Kelley (EOSG) <michael.h.kel...@microsoft.com>; vkuzn...@redhat.com > Cc: KY Srinivasan <k...@microsoft.com> > Subject: [PATCH V2 0/5] X86: Hyper-V: APIC enlightenments > > From: "K. Y. Srinivasan" <k...@microsoft.com> > > Implement APIC related enlightenments. > > V2: Addressed comments from Thomas Gleixner <t...@linutronix.de> > and Michael Kelley (EOSG) <michael.h.kel...@microsoft.com>. Thomas, I think I have addressed all your comments; let me know if you have any additional comments on this patch set. Regards, K. Y > > K. Y. Srinivasan (5): > X86: Hyper-V: Enlighten APIC access > X86: Hyper-V: Enable IPI enlightenments > X86: Hyper-V: Enhanced IPI enlightenment > X86: Hyper-V: Consolidate code for converting cpumask to vpset > X86: Hyper-V: Consolidate the allocation of the hypercall input page > > arch/x86/hyperv/Makefile | 2 +- > arch/x86/hyperv/hv_apic.c | 259 > + > arch/x86/hyperv/hv_init.c | 32 - > arch/x86/hyperv/mmu.c | 75 ++- > arch/x86/include/asm/hyperv-tlfs.h | 30 - > arch/x86/include/asm/mshyperv.h| 39 +- > 6 files changed, 365 insertions(+), 72 deletions(-) > create mode 100644 arch/x86/hyperv/hv_apic.c > > -- > 2.15.1
RE: [PATCH V2 0/5] X86: Hyper-V: APIC enlightenments
> -Original Message- > From: k...@linuxonhyperv.com > Sent: Thursday, May 3, 2018 11:07 PM > To: 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; > h...@zytor.com; Stephen Hemminger ; Michael > Kelley (EOSG) ; vkuzn...@redhat.com > Cc: KY Srinivasan > Subject: [PATCH V2 0/5] X86: Hyper-V: APIC enlightenments > > From: "K. Y. Srinivasan" > > Implement APIC related enlightenments. > > V2: Addressed comments from Thomas Gleixner > and Michael Kelley (EOSG) . Thomas, I think I have addressed all your comments; let me know if you have any additional comments on this patch set. Regards, K. Y > > K. Y. Srinivasan (5): > X86: Hyper-V: Enlighten APIC access > X86: Hyper-V: Enable IPI enlightenments > X86: Hyper-V: Enhanced IPI enlightenment > X86: Hyper-V: Consolidate code for converting cpumask to vpset > X86: Hyper-V: Consolidate the allocation of the hypercall input page > > arch/x86/hyperv/Makefile | 2 +- > arch/x86/hyperv/hv_apic.c | 259 > + > arch/x86/hyperv/hv_init.c | 32 - > arch/x86/hyperv/mmu.c | 75 ++- > arch/x86/include/asm/hyperv-tlfs.h | 30 - > arch/x86/include/asm/mshyperv.h| 39 +- > 6 files changed, 365 insertions(+), 72 deletions(-) > create mode 100644 arch/x86/hyperv/hv_apic.c > > -- > 2.15.1
RE: [PATCH v4 1/8] x86/hyper-v: move struct hv_flush_pcpu{,ex} definitions to common header
> -Original Message- > From: Vitaly Kuznetsov <vkuzn...@redhat.com> > Sent: Wednesday, May 16, 2018 8:21 AM > To: k...@vger.kernel.org > Cc: x...@kernel.org; Paolo Bonzini <pbonz...@redhat.com>; Radim Krčmář > <rkrc...@redhat.com>; Roman Kagan <rka...@virtuozzo.com>; KY > Srinivasan <k...@microsoft.com>; Haiyang Zhang > <haiya...@microsoft.com>; Stephen Hemminger > <sthem...@microsoft.com>; Michael Kelley (EOSG) > <michael.h.kel...@microsoft.com>; Mohammed Gamal > <mmo...@redhat.com>; Cathy Avery <cav...@redhat.com>; linux- > ker...@vger.kernel.org > Subject: [PATCH v4 1/8] x86/hyper-v: move struct hv_flush_pcpu{,ex} > definitions to common header > > Hyper-V TLB flush hypercalls definitions will be required for KVM so move > them hyperv-tlfs.h. Structures also need to be renamed as '_pcpu' suffix is > irrelevant for a general-purpose definition. > > Signed-off-by: Vitaly Kuznetsov <vkuzn...@redhat.com> > --- > arch/x86/hyperv/mmu.c | 40 > ++ > arch/x86/include/asm/hyperv-tlfs.h | 20 +++ > 2 files changed, 30 insertions(+), 30 deletions(-) Vitaly, We should coordinate on this; I have patches in flight that conflict with the changes here. Regards, K. Y > > diff --git a/arch/x86/hyperv/mmu.c b/arch/x86/hyperv/mmu.c > index 56c9ebac946f..528a1f34df96 100644 > --- a/arch/x86/hyperv/mmu.c > +++ b/arch/x86/hyperv/mmu.c > @@ -13,32 +13,12 @@ > #define CREATE_TRACE_POINTS > #include > > -/* HvFlushVirtualAddressSpace, HvFlushVirtualAddressList hypercalls */ > -struct hv_flush_pcpu { > - u64 address_space; > - u64 flags; > - u64 processor_mask; > - u64 gva_list[]; > -}; > - > -/* HvFlushVirtualAddressSpaceEx, HvFlushVirtualAddressListEx hypercalls */ > -struct hv_flush_pcpu_ex { > - u64 address_space; > - u64 flags; > - struct { > - u64 format; > - u64 valid_bank_mask; > - u64 bank_contents[]; > - } hv_vp_set; > - u64 gva_list[]; > -}; > - > /* Each gva in gva_list encodes up to 4096 pages to flush */ > #define HV_TLB_FLUSH_UNIT (4096 * PAGE_SIZE) > > -static struct hv_flush_pcpu __percpu **pcpu_flush; > +static struct hv_tlb_flush * __percpu *pcpu_flush; > > -static struct hv_flush_pcpu_ex __percpu **pcpu_flush_ex; > +static struct hv_tlb_flush_ex * __percpu *pcpu_flush_ex; > > /* > * Fills in gva_list starting from offset. Returns the number of items added. > @@ -71,7 +51,7 @@ static inline int fill_gva_list(u64 gva_list[], int offset, > } > > /* Return the number of banks in the resulting vp_set */ > -static inline int cpumask_to_vp_set(struct hv_flush_pcpu_ex *flush, > +static inline int cpumask_to_vp_set(struct hv_tlb_flush_ex *flush, > const struct cpumask *cpus) > { > int cpu, vcpu, vcpu_bank, vcpu_offset, nr_bank = 1; > @@ -81,7 +61,7 @@ static inline int cpumask_to_vp_set(struct > hv_flush_pcpu_ex *flush, > return 0; > > /* > - * Clear all banks up to the maximum possible bank as > hv_flush_pcpu_ex > + * Clear all banks up to the maximum possible bank as > hv_tlb_flush_ex >* structs are not cleared between calls, we risk flushing unneeded >* vCPUs otherwise. >*/ > @@ -109,8 +89,8 @@ static void hyperv_flush_tlb_others(const struct > cpumask *cpus, > const struct flush_tlb_info *info) > { > int cpu, vcpu, gva_n, max_gvas; > - struct hv_flush_pcpu **flush_pcpu; > - struct hv_flush_pcpu *flush; > + struct hv_tlb_flush **flush_pcpu; > + struct hv_tlb_flush *flush; > u64 status = U64_MAX; > unsigned long flags; > > @@ -196,8 +176,8 @@ static void hyperv_flush_tlb_others_ex(const struct > cpumask *cpus, > const struct flush_tlb_info *info) > { > int nr_bank = 0, max_gvas, gva_n; > - struct hv_flush_pcpu_ex **flush_pcpu; > - struct hv_flush_pcpu_ex *flush; > + struct hv_tlb_flush_ex **flush_pcpu; > + struct hv_tlb_flush_ex *flush; > u64 status = U64_MAX; > unsigned long flags; > > @@ -303,7 +283,7 @@ void hyper_alloc_mmu(void) > return; > > if (!(ms_hyperv.hints & > HV_X64_EX_PROCESSOR_MASKS_RECOMMENDED)) > - pcpu_flush = alloc_percpu(struct hv_flush_pcpu *); > + pcpu_flush = alloc_percpu(struct hv_tlb_flush *); > else > - pcpu_flush_ex = alloc_percpu(struct hv_flush_pcpu_ex *); > + pcpu_flush_ex = alloc_
RE: [PATCH v4 1/8] x86/hyper-v: move struct hv_flush_pcpu{,ex} definitions to common header
> -Original Message- > From: Vitaly Kuznetsov > Sent: Wednesday, May 16, 2018 8:21 AM > To: k...@vger.kernel.org > Cc: x...@kernel.org; Paolo Bonzini ; Radim Krčmář > ; Roman Kagan ; KY > Srinivasan ; Haiyang Zhang > ; Stephen Hemminger > ; Michael Kelley (EOSG) > ; Mohammed Gamal > ; Cathy Avery ; linux- > ker...@vger.kernel.org > Subject: [PATCH v4 1/8] x86/hyper-v: move struct hv_flush_pcpu{,ex} > definitions to common header > > Hyper-V TLB flush hypercalls definitions will be required for KVM so move > them hyperv-tlfs.h. Structures also need to be renamed as '_pcpu' suffix is > irrelevant for a general-purpose definition. > > Signed-off-by: Vitaly Kuznetsov > --- > arch/x86/hyperv/mmu.c | 40 > ++ > arch/x86/include/asm/hyperv-tlfs.h | 20 +++ > 2 files changed, 30 insertions(+), 30 deletions(-) Vitaly, We should coordinate on this; I have patches in flight that conflict with the changes here. Regards, K. Y > > diff --git a/arch/x86/hyperv/mmu.c b/arch/x86/hyperv/mmu.c > index 56c9ebac946f..528a1f34df96 100644 > --- a/arch/x86/hyperv/mmu.c > +++ b/arch/x86/hyperv/mmu.c > @@ -13,32 +13,12 @@ > #define CREATE_TRACE_POINTS > #include > > -/* HvFlushVirtualAddressSpace, HvFlushVirtualAddressList hypercalls */ > -struct hv_flush_pcpu { > - u64 address_space; > - u64 flags; > - u64 processor_mask; > - u64 gva_list[]; > -}; > - > -/* HvFlushVirtualAddressSpaceEx, HvFlushVirtualAddressListEx hypercalls */ > -struct hv_flush_pcpu_ex { > - u64 address_space; > - u64 flags; > - struct { > - u64 format; > - u64 valid_bank_mask; > - u64 bank_contents[]; > - } hv_vp_set; > - u64 gva_list[]; > -}; > - > /* Each gva in gva_list encodes up to 4096 pages to flush */ > #define HV_TLB_FLUSH_UNIT (4096 * PAGE_SIZE) > > -static struct hv_flush_pcpu __percpu **pcpu_flush; > +static struct hv_tlb_flush * __percpu *pcpu_flush; > > -static struct hv_flush_pcpu_ex __percpu **pcpu_flush_ex; > +static struct hv_tlb_flush_ex * __percpu *pcpu_flush_ex; > > /* > * Fills in gva_list starting from offset. Returns the number of items added. > @@ -71,7 +51,7 @@ static inline int fill_gva_list(u64 gva_list[], int offset, > } > > /* Return the number of banks in the resulting vp_set */ > -static inline int cpumask_to_vp_set(struct hv_flush_pcpu_ex *flush, > +static inline int cpumask_to_vp_set(struct hv_tlb_flush_ex *flush, > const struct cpumask *cpus) > { > int cpu, vcpu, vcpu_bank, vcpu_offset, nr_bank = 1; > @@ -81,7 +61,7 @@ static inline int cpumask_to_vp_set(struct > hv_flush_pcpu_ex *flush, > return 0; > > /* > - * Clear all banks up to the maximum possible bank as > hv_flush_pcpu_ex > + * Clear all banks up to the maximum possible bank as > hv_tlb_flush_ex >* structs are not cleared between calls, we risk flushing unneeded >* vCPUs otherwise. >*/ > @@ -109,8 +89,8 @@ static void hyperv_flush_tlb_others(const struct > cpumask *cpus, > const struct flush_tlb_info *info) > { > int cpu, vcpu, gva_n, max_gvas; > - struct hv_flush_pcpu **flush_pcpu; > - struct hv_flush_pcpu *flush; > + struct hv_tlb_flush **flush_pcpu; > + struct hv_tlb_flush *flush; > u64 status = U64_MAX; > unsigned long flags; > > @@ -196,8 +176,8 @@ static void hyperv_flush_tlb_others_ex(const struct > cpumask *cpus, > const struct flush_tlb_info *info) > { > int nr_bank = 0, max_gvas, gva_n; > - struct hv_flush_pcpu_ex **flush_pcpu; > - struct hv_flush_pcpu_ex *flush; > + struct hv_tlb_flush_ex **flush_pcpu; > + struct hv_tlb_flush_ex *flush; > u64 status = U64_MAX; > unsigned long flags; > > @@ -303,7 +283,7 @@ void hyper_alloc_mmu(void) > return; > > if (!(ms_hyperv.hints & > HV_X64_EX_PROCESSOR_MASKS_RECOMMENDED)) > - pcpu_flush = alloc_percpu(struct hv_flush_pcpu *); > + pcpu_flush = alloc_percpu(struct hv_tlb_flush *); > else > - pcpu_flush_ex = alloc_percpu(struct hv_flush_pcpu_ex *); > + pcpu_flush_ex = alloc_percpu(struct hv_tlb_flush_ex *); > } > diff --git a/arch/x86/include/asm/hyperv-tlfs.h > b/arch/x86/include/asm/hyperv-tlfs.h > index a8897615354e..3d4ce3935a62 100644 > --- a/arch/x86/include/asm/hyperv-tlfs.h > +++ b/arch/x86/include/asm/hyperv-tlfs.h
RE: [PATCH char-misc 1/2] Drivers: hv: vmbus: Remove x86 MSR refs in arch independent code
> -Original Message- > From: mhkelle...@gmail.com <mhkelle...@gmail.com> > Sent: Tuesday, May 8, 2018 8:38 AM > To: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > vkuzn...@redhat.com; jasow...@redhat.com; > leann.ogasaw...@canonical.com; marcelo.ce...@canonical.com; Stephen > Hemminger <sthem...@microsoft.com>; KY Srinivasan > <k...@microsoft.com> > Subject: [PATCH char-misc 1/2] Drivers: hv: vmbus: Remove x86 MSR refs in > arch independent code > > From: Michael Kelley <mikel...@microsoft.com> > > In architecture independent code for manipulating Hyper-V synthetic timers > and synthetic interrupts, pass in an ordinal number identifying the timer > or interrupt, rather than an actual MSR register address. Then in > x86/x64 specific code, map the ordinal number to the appropriate MSR. > This change facilitates the introduction of an ARM64 version of Hyper-V, > which uses the same synthetic timers and interrupts, but a different > mechanism for accessing them. > > Signed-off-by: Michael Kelley <mikel...@microsoft.com> > --- > arch/x86/include/asm/mshyperv.h | 12 > drivers/hv/hv.c | 20 > 2 files changed, 16 insertions(+), 16 deletions(-) > > diff --git a/arch/x86/include/asm/mshyperv.h > b/arch/x86/include/asm/mshyperv.h > index b90e796..caf9035 100644 > --- a/arch/x86/include/asm/mshyperv.h > +++ b/arch/x86/include/asm/mshyperv.h > @@ -75,8 +75,10 @@ static inline void vmbus_signal_eom(struct > hv_message *msg, u32 old_msg_type) > } > } > > -#define hv_init_timer(timer, tick) wrmsrl(timer, tick) > -#define hv_init_timer_config(config, val) wrmsrl(config, val) > +#define hv_init_timer(timer, tick) \ > + wrmsrl(HV_X64_MSR_STIMER0_COUNT + (2*timer), tick) > +#define hv_init_timer_config(timer, val) \ > + wrmsrl(HV_X64_MSR_STIMER0_CONFIG + (2*timer), val) Why are we stepping in units of 2? K. Y
RE: [PATCH char-misc 1/2] Drivers: hv: vmbus: Remove x86 MSR refs in arch independent code
> -Original Message- > From: mhkelle...@gmail.com > Sent: Tuesday, May 8, 2018 8:38 AM > To: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > vkuzn...@redhat.com; jasow...@redhat.com; > leann.ogasaw...@canonical.com; marcelo.ce...@canonical.com; Stephen > Hemminger ; KY Srinivasan > > Subject: [PATCH char-misc 1/2] Drivers: hv: vmbus: Remove x86 MSR refs in > arch independent code > > From: Michael Kelley > > In architecture independent code for manipulating Hyper-V synthetic timers > and synthetic interrupts, pass in an ordinal number identifying the timer > or interrupt, rather than an actual MSR register address. Then in > x86/x64 specific code, map the ordinal number to the appropriate MSR. > This change facilitates the introduction of an ARM64 version of Hyper-V, > which uses the same synthetic timers and interrupts, but a different > mechanism for accessing them. > > Signed-off-by: Michael Kelley > --- > arch/x86/include/asm/mshyperv.h | 12 > drivers/hv/hv.c | 20 > 2 files changed, 16 insertions(+), 16 deletions(-) > > diff --git a/arch/x86/include/asm/mshyperv.h > b/arch/x86/include/asm/mshyperv.h > index b90e796..caf9035 100644 > --- a/arch/x86/include/asm/mshyperv.h > +++ b/arch/x86/include/asm/mshyperv.h > @@ -75,8 +75,10 @@ static inline void vmbus_signal_eom(struct > hv_message *msg, u32 old_msg_type) > } > } > > -#define hv_init_timer(timer, tick) wrmsrl(timer, tick) > -#define hv_init_timer_config(config, val) wrmsrl(config, val) > +#define hv_init_timer(timer, tick) \ > + wrmsrl(HV_X64_MSR_STIMER0_COUNT + (2*timer), tick) > +#define hv_init_timer_config(timer, val) \ > + wrmsrl(HV_X64_MSR_STIMER0_CONFIG + (2*timer), val) Why are we stepping in units of 2? K. Y
RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments
> -Original Message- > From: Dan Carpenter <dan.carpen...@oracle.com> > Sent: Thursday, April 26, 2018 2:32 PM > To: KY Srinivasan <k...@microsoft.com> > 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; > h...@zytor.com; Stephen Hemminger <sthem...@microsoft.com>; Michael > Kelley (EOSG) <michael.h.kel...@microsoft.com>; vkuzn...@redhat.com > Subject: Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments > > On Wed, Apr 25, 2018 at 11:12:47AM -0700, k...@linuxonhyperv.com wrote: > > +/* > > + * IPI implementation on Hyper-V. > > + */ > > + > > +static int __send_ipi_mask(const struct cpumask *mask, int vector) > > +{ > > + int cur_cpu, vcpu; > > + struct ipi_arg_non_ex **arg; > > + struct ipi_arg_non_ex *ipi_arg; > > + int ret = 1; > > Not specifically related to this patch, but hv code sometimes returns 1 > on error or U64_MAX. It's slightly magical. Maybe > HV_STATUS_INVALID_HYPERCALL_INPUT (3) would be more appropriate? > Or we > could make a new more generic error code: > > #define HV_STATUS_INVALID1 Good point. We will look at cleaning this up. Regards, K. Y
RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments
> -Original Message- > From: Dan Carpenter > Sent: Thursday, April 26, 2018 2:32 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; > h...@zytor.com; Stephen Hemminger ; Michael > Kelley (EOSG) ; vkuzn...@redhat.com > Subject: Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments > > On Wed, Apr 25, 2018 at 11:12:47AM -0700, k...@linuxonhyperv.com wrote: > > +/* > > + * IPI implementation on Hyper-V. > > + */ > > + > > +static int __send_ipi_mask(const struct cpumask *mask, int vector) > > +{ > > + int cur_cpu, vcpu; > > + struct ipi_arg_non_ex **arg; > > + struct ipi_arg_non_ex *ipi_arg; > > + int ret = 1; > > Not specifically related to this patch, but hv code sometimes returns 1 > on error or U64_MAX. It's slightly magical. Maybe > HV_STATUS_INVALID_HYPERCALL_INPUT (3) would be more appropriate? > Or we > could make a new more generic error code: > > #define HV_STATUS_INVALID1 Good point. We will look at cleaning this up. Regards, K. Y
RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments
> -Original Message- > From: Michael Kelley (EOSG) > Sent: Thursday, April 26, 2018 3:55 PM > To: KY Srinivasan <k...@microsoft.com>; 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; Stephen > Hemminger <sthem...@microsoft.com>; vkuzn...@redhat.com > Subject: RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments > > > -Original Message- > > From: k...@linuxonhyperv.com <k...@linuxonhyperv.com> > > Sent: Wednesday, April 25, 2018 11:13 AM > > To: 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; h...@zytor.com; Stephen Hemminger > <sthem...@microsoft.com>; > > Michael Kelley (EOSG) <michael.h.kel...@microsoft.com>; > vkuzn...@redhat.com > > Cc: KY Srinivasan <k...@microsoft.com> > > Subject: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments > > > > From: "K. Y. Srinivasan" <k...@microsoft.com> > > > > Hyper-V supports hypercalls to implement IPI; use them. > > > > Signed-off-by: K. Y. Srinivasan <k...@microsoft.com> > > --- > > arch/x86/hyperv/hv_apic.c | 125 > + > > arch/x86/hyperv/hv_init.c | 17 + > > arch/x86/include/asm/hyperv-tlfs.h | 9 +++ > > arch/x86/include/asm/mshyperv.h| 1 + > > 4 files changed, 152 insertions(+) > > > > diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c > > index e0a5b36208fc..7f3322ecfb01 100644 > > --- a/arch/x86/hyperv/hv_apic.c > > +++ b/arch/x86/hyperv/hv_apic.c > > @@ -30,6 +30,14 @@ > > #include > > #include > > > > +struct ipi_arg_non_ex { > > + u32 vector; > > + u32 reserved; > > + u64 cpu_mask; > > +}; > > I think we'd like to put structures like this, which are defined in the > Hyper-V Top Level Functional Spec, in hyperv-tlfs.h. Also, the 5.0b > version of the TLFS, which is latest, shows this structure on page 100: > > u32 vector; > u8 targetvtl; > u8 reserved[3]; > u64 cpu_mask; > Good point. I will make the necessary adjustments. Regards, K. Y
RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments
> -Original Message- > From: Michael Kelley (EOSG) > Sent: Thursday, April 26, 2018 3:55 PM > To: KY Srinivasan ; 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; Stephen > Hemminger ; vkuzn...@redhat.com > Subject: RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments > > > -Original Message- > > From: k...@linuxonhyperv.com > > Sent: Wednesday, April 25, 2018 11:13 AM > > To: 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; h...@zytor.com; Stephen Hemminger > ; > > Michael Kelley (EOSG) ; > vkuzn...@redhat.com > > Cc: KY Srinivasan > > Subject: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments > > > > From: "K. Y. Srinivasan" > > > > Hyper-V supports hypercalls to implement IPI; use them. > > > > Signed-off-by: K. Y. Srinivasan > > --- > > arch/x86/hyperv/hv_apic.c | 125 > + > > arch/x86/hyperv/hv_init.c | 17 + > > arch/x86/include/asm/hyperv-tlfs.h | 9 +++ > > arch/x86/include/asm/mshyperv.h| 1 + > > 4 files changed, 152 insertions(+) > > > > diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c > > index e0a5b36208fc..7f3322ecfb01 100644 > > --- a/arch/x86/hyperv/hv_apic.c > > +++ b/arch/x86/hyperv/hv_apic.c > > @@ -30,6 +30,14 @@ > > #include > > #include > > > > +struct ipi_arg_non_ex { > > + u32 vector; > > + u32 reserved; > > + u64 cpu_mask; > > +}; > > I think we'd like to put structures like this, which are defined in the > Hyper-V Top Level Functional Spec, in hyperv-tlfs.h. Also, the 5.0b > version of the TLFS, which is latest, shows this structure on page 100: > > u32 vector; > u8 targetvtl; > u8 reserved[3]; > u64 cpu_mask; > Good point. I will make the necessary adjustments. Regards, K. Y
RE: [PATCH 5/5] X86: Hyper-V: Consolidate the allocation of the hypercall input page
> -Original Message- > From: Thomas Gleixner <t...@linutronix.de> > Sent: Thursday, April 26, 2018 3:24 PM > To: KY Srinivasan <k...@microsoft.com> > Cc: x...@kernel.org; gre...@linuxfoundation.org; linux- > ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de; > a...@canonical.com; jasow...@redhat.com; h...@zytor.com; Stephen > Hemminger <sthem...@microsoft.com>; Michael Kelley (EOSG) > <michael.h.kel...@microsoft.com>; vkuzn...@redhat.com > Subject: Re: [PATCH 5/5] X86: Hyper-V: Consolidate the allocation of the > hypercall input page > > On Wed, 25 Apr 2018, k...@linuxonhyperv.com wrote: > > > From: "K. Y. Srinivasan" <k...@microsoft.com> > > > > Consolidate the allocation of the hypercall input page. > > Again. You can provide the new way of allocation first, then you don't have > to add code first in order to remove it later again. I have implemented the new way upfront for the new code - the IPI code [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments. What I am doing here using that infrastructure for the TLB flush enlightenments and getting rid of unnecessary code. Regards, K. Y
RE: [PATCH 5/5] X86: Hyper-V: Consolidate the allocation of the hypercall input page
> -Original Message- > From: Thomas Gleixner > Sent: Thursday, April 26, 2018 3:24 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; h...@zytor.com; Stephen > Hemminger ; Michael Kelley (EOSG) > ; vkuzn...@redhat.com > Subject: Re: [PATCH 5/5] X86: Hyper-V: Consolidate the allocation of the > hypercall input page > > On Wed, 25 Apr 2018, k...@linuxonhyperv.com wrote: > > > From: "K. Y. Srinivasan" > > > > Consolidate the allocation of the hypercall input page. > > Again. You can provide the new way of allocation first, then you don't have > to add code first in order to remove it later again. I have implemented the new way upfront for the new code - the IPI code [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments. What I am doing here using that infrastructure for the TLB flush enlightenments and getting rid of unnecessary code. Regards, K. Y
RE: [PATCH 3/5] X86: Hyper-V: Enhanced IPI enlightenment
> -Original Message- > From: Thomas Gleixner <t...@linutronix.de> > Sent: Thursday, April 26, 2018 3:17 PM > To: KY Srinivasan <k...@microsoft.com> > Cc: x...@kernel.org; gre...@linuxfoundation.org; linux- > ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de; > a...@canonical.com; jasow...@redhat.com; h...@zytor.com; Stephen > Hemminger <sthem...@microsoft.com>; Michael Kelley (EOSG) > <michael.h.kel...@microsoft.com>; vkuzn...@redhat.com > Subject: Re: [PATCH 3/5] X86: Hyper-V: Enhanced IPI enlightenment > > On Wed, 25 Apr 2018, k...@linuxonhyperv.com wrote: > > > > +struct ipi_arg_ex { > > + u32 vector; > > + u32 reserved; > > + struct hv_vpset vp_set; > > Please align that in tabular fashion for easy of reading > > u32 vector; > u32 reserved; > struct hv_vpset vp_set; > > > +}; > > + > > static struct apic orig_apic; > > > > static u64 hv_apic_icr_read(void) > > @@ -97,6 +103,40 @@ static void hv_apic_eoi_write(u32 reg, u32 val) > > * IPI implementation on Hyper-V. > > */ > > > > +static int __send_ipi_mask_ex(const struct cpumask *mask, int vector) > > +{ > > + int nr_bank = 0; > > + struct ipi_arg_ex **arg; > > + struct ipi_arg_ex *ipi_arg; > > + int ret = 1; > > + unsigned long flags; > > This is really horrible to read. > > struct ipi_arg_ex *ipi_arg; > struct ipi_arg_ex **arg; > unsigned long flags; > bool ret = false; > int nr_bank = 0; > > is really more conveniant for quick reading. > > So the other more limited function has a lot more sanity checks vs. vector > number and other things. Why are they not required here? Comment > please. Yes, I will add the comments. This function is called from the other function after all the sanity checks have been done and hence are not replicated here. > > > + local_irq_save(flags); > > + arg = (struct ipi_arg_ex **)this_cpu_ptr(hyperv_pcpu_input_arg); > > + > > + ipi_arg = *arg; > > + if (unlikely(!ipi_arg)) > > + goto ipi_mask_ex_done; > > + > > + ipi_arg->vector = vector; > > + ipi_arg->reserved = 0; > > + ipi_arg->vp_set.valid_bank_mask = 0; > > + > > + if (!cpumask_equal(mask, cpu_present_mask)) { > > + ipi_arg->vp_set.format = HV_GENERIC_SET_SPARCE_4K; > > + nr_bank = cpumask_to_vpset(&(ipi_arg->vp_set), mask); > > nr_bank really confused me. bank_nr is what you mean, not number of > banks, > right? It is the number of banks. The hypercall used here is a variable length hypercall. Regards, K. Y
RE: [PATCH 3/5] X86: Hyper-V: Enhanced IPI enlightenment
> -Original Message- > From: Thomas Gleixner > Sent: Thursday, April 26, 2018 3:17 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; h...@zytor.com; Stephen > Hemminger ; Michael Kelley (EOSG) > ; vkuzn...@redhat.com > Subject: Re: [PATCH 3/5] X86: Hyper-V: Enhanced IPI enlightenment > > On Wed, 25 Apr 2018, k...@linuxonhyperv.com wrote: > > > > +struct ipi_arg_ex { > > + u32 vector; > > + u32 reserved; > > + struct hv_vpset vp_set; > > Please align that in tabular fashion for easy of reading > > u32 vector; > u32 reserved; > struct hv_vpset vp_set; > > > +}; > > + > > static struct apic orig_apic; > > > > static u64 hv_apic_icr_read(void) > > @@ -97,6 +103,40 @@ static void hv_apic_eoi_write(u32 reg, u32 val) > > * IPI implementation on Hyper-V. > > */ > > > > +static int __send_ipi_mask_ex(const struct cpumask *mask, int vector) > > +{ > > + int nr_bank = 0; > > + struct ipi_arg_ex **arg; > > + struct ipi_arg_ex *ipi_arg; > > + int ret = 1; > > + unsigned long flags; > > This is really horrible to read. > > struct ipi_arg_ex *ipi_arg; > struct ipi_arg_ex **arg; > unsigned long flags; > bool ret = false; > int nr_bank = 0; > > is really more conveniant for quick reading. > > So the other more limited function has a lot more sanity checks vs. vector > number and other things. Why are they not required here? Comment > please. Yes, I will add the comments. This function is called from the other function after all the sanity checks have been done and hence are not replicated here. > > > + local_irq_save(flags); > > + arg = (struct ipi_arg_ex **)this_cpu_ptr(hyperv_pcpu_input_arg); > > + > > + ipi_arg = *arg; > > + if (unlikely(!ipi_arg)) > > + goto ipi_mask_ex_done; > > + > > + ipi_arg->vector = vector; > > + ipi_arg->reserved = 0; > > + ipi_arg->vp_set.valid_bank_mask = 0; > > + > > + if (!cpumask_equal(mask, cpu_present_mask)) { > > + ipi_arg->vp_set.format = HV_GENERIC_SET_SPARCE_4K; > > + nr_bank = cpumask_to_vpset(&(ipi_arg->vp_set), mask); > > nr_bank really confused me. bank_nr is what you mean, not number of > banks, > right? It is the number of banks. The hypercall used here is a variable length hypercall. Regards, K. Y
RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments
> -Original Message- > From: Thomas Gleixner <t...@linutronix.de> > Sent: Thursday, April 26, 2018 3:09 PM > To: KY Srinivasan <k...@microsoft.com> > Cc: x...@kernel.org; gre...@linuxfoundation.org; linux- > ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de; > a...@canonical.com; jasow...@redhat.com; h...@zytor.com; Stephen > Hemminger <sthem...@microsoft.com>; Michael Kelley (EOSG) > <michael.h.kel...@microsoft.com>; vkuzn...@redhat.com > Subject: Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments > > On Wed, 25 Apr 2018, k...@linuxonhyperv.com wrote: > > +static int __send_ipi_mask(const struct cpumask *mask, int vector) > > +{ > > + int cur_cpu, vcpu; > > + struct ipi_arg_non_ex **arg; > > + struct ipi_arg_non_ex *ipi_arg; > > + int ret = 1; > > So this indicates whether __send_ipi_mask() can send to @mask or not. So > please make it a bool and let it return false when it does not work, true > otherwise. If you had used -E then it would have been more obvious, > but > this is really a boolean decision. Agreed. > > > + unsigned long flags; > > + > > + if (cpumask_empty(mask)) > > + return 0; > > + > > + if (!hv_hypercall_pg) > > + return ret; > > + > > + if ((vector < HV_IPI_LOW_VECTOR) || (vector > > HV_IPI_HIGH_VECTOR)) > > + return ret; > > + > > + local_irq_save(flags); > > + arg = (struct ipi_arg_non_ex > **)this_cpu_ptr(hyperv_pcpu_input_arg); > > + > > + ipi_arg = *arg; > > + if (unlikely(!ipi_arg)) > > + goto ipi_mask_done; > > + > > + > > Stray newline > > > + ipi_arg->vector = vector; > > + ipi_arg->reserved = 0; > > + ipi_arg->cpu_mask = 0; > > + > > + for_each_cpu(cur_cpu, mask) { > > + vcpu = hv_cpu_number_to_vp_number(cur_cpu); > > + if (vcpu >= 64) > > + goto ipi_mask_done; > > This is completely magic and deserves a comment. > > > + > > + __set_bit(vcpu, (unsigned long *)_arg->cpu_mask); > > + } > > + > > + ret = hv_do_hypercall(HVCALL_SEND_IPI, ipi_arg, NULL); > > + > > +ipi_mask_done: > > + local_irq_restore(flags); > > + return ret; > > +} > > > > > static int hv_cpu_init(unsigned int cpu) > > { > > u64 msr_vp_index; > > struct hv_vp_assist_page **hvp = > _vp_assist_page[smp_processor_id()]; > > + void **input_arg; > > + > > + input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg); > > + *input_arg = page_address(alloc_page(GFP_ATOMIC)); > > This is called from the cpu hotplug thread and there is no need for an > atomic allocation. Please use GFP_KERNEL. > > > hv_get_vp_index(msr_vp_index); > > > > @@ -217,6 +224,10 @@ static int hv_cpu_die(unsigned int cpu) > > { > > struct hv_reenlightenment_control re_ctrl; > > unsigned int new_cpu; > > + void **input_arg; > > + > > + input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg); > > + free_page((unsigned long)*input_arg); > > Hrm. Again this is called from the CPU hotplug thread when the cou is about > to go down. But you can be scheduled out after free() and before disabling > the assist thing below and the pointer persist. There is no guarantee that > nothing sends an IPI anymore after this point. > > So you have two options here: > > 1) Disable interrupts, get the pointer, set the per cpu pointer to NULL, > reenable interruots and free the page I will implement this approach. > > 2) Keep the page around and check for it in the CPU UP path and avoid the > allocation when the CPU comes online again. > > > if (hv_vp_assist_page && hv_vp_assist_page[cpu]) > > wrmsrl(HV_X64_MSR_VP_ASSIST_PAGE, 0); > > @@ -260,6 +271,12 @@ void __init hyperv_init(void) > > if ((ms_hyperv.features & required_msrs) != required_msrs) > > return; > > > > + /* Allocate the per-CPU state for the hypercall input arg */ > > + hyperv_pcpu_input_arg = alloc_percpu(void *); > > + > > + if (hyperv_pcpu_input_arg == NULL) > > + return; > > Huch. When that allocation fails, you return and ignore the rest of the > function which has been there before. Weird decision. I should have explained this. Failure of this allocation means that we would not have the per-cpu hypercall input page which in turn would mean that we would not be able to invoke any hypercalls. Regards, K. Y
RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments
> -Original Message- > From: Thomas Gleixner > Sent: Thursday, April 26, 2018 3:09 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; h...@zytor.com; Stephen > Hemminger ; Michael Kelley (EOSG) > ; vkuzn...@redhat.com > Subject: Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments > > On Wed, 25 Apr 2018, k...@linuxonhyperv.com wrote: > > +static int __send_ipi_mask(const struct cpumask *mask, int vector) > > +{ > > + int cur_cpu, vcpu; > > + struct ipi_arg_non_ex **arg; > > + struct ipi_arg_non_ex *ipi_arg; > > + int ret = 1; > > So this indicates whether __send_ipi_mask() can send to @mask or not. So > please make it a bool and let it return false when it does not work, true > otherwise. If you had used -E then it would have been more obvious, > but > this is really a boolean decision. Agreed. > > > + unsigned long flags; > > + > > + if (cpumask_empty(mask)) > > + return 0; > > + > > + if (!hv_hypercall_pg) > > + return ret; > > + > > + if ((vector < HV_IPI_LOW_VECTOR) || (vector > > HV_IPI_HIGH_VECTOR)) > > + return ret; > > + > > + local_irq_save(flags); > > + arg = (struct ipi_arg_non_ex > **)this_cpu_ptr(hyperv_pcpu_input_arg); > > + > > + ipi_arg = *arg; > > + if (unlikely(!ipi_arg)) > > + goto ipi_mask_done; > > + > > + > > Stray newline > > > + ipi_arg->vector = vector; > > + ipi_arg->reserved = 0; > > + ipi_arg->cpu_mask = 0; > > + > > + for_each_cpu(cur_cpu, mask) { > > + vcpu = hv_cpu_number_to_vp_number(cur_cpu); > > + if (vcpu >= 64) > > + goto ipi_mask_done; > > This is completely magic and deserves a comment. > > > + > > + __set_bit(vcpu, (unsigned long *)_arg->cpu_mask); > > + } > > + > > + ret = hv_do_hypercall(HVCALL_SEND_IPI, ipi_arg, NULL); > > + > > +ipi_mask_done: > > + local_irq_restore(flags); > > + return ret; > > +} > > > > > static int hv_cpu_init(unsigned int cpu) > > { > > u64 msr_vp_index; > > struct hv_vp_assist_page **hvp = > _vp_assist_page[smp_processor_id()]; > > + void **input_arg; > > + > > + input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg); > > + *input_arg = page_address(alloc_page(GFP_ATOMIC)); > > This is called from the cpu hotplug thread and there is no need for an > atomic allocation. Please use GFP_KERNEL. > > > hv_get_vp_index(msr_vp_index); > > > > @@ -217,6 +224,10 @@ static int hv_cpu_die(unsigned int cpu) > > { > > struct hv_reenlightenment_control re_ctrl; > > unsigned int new_cpu; > > + void **input_arg; > > + > > + input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg); > > + free_page((unsigned long)*input_arg); > > Hrm. Again this is called from the CPU hotplug thread when the cou is about > to go down. But you can be scheduled out after free() and before disabling > the assist thing below and the pointer persist. There is no guarantee that > nothing sends an IPI anymore after this point. > > So you have two options here: > > 1) Disable interrupts, get the pointer, set the per cpu pointer to NULL, > reenable interruots and free the page I will implement this approach. > > 2) Keep the page around and check for it in the CPU UP path and avoid the > allocation when the CPU comes online again. > > > if (hv_vp_assist_page && hv_vp_assist_page[cpu]) > > wrmsrl(HV_X64_MSR_VP_ASSIST_PAGE, 0); > > @@ -260,6 +271,12 @@ void __init hyperv_init(void) > > if ((ms_hyperv.features & required_msrs) != required_msrs) > > return; > > > > + /* Allocate the per-CPU state for the hypercall input arg */ > > + hyperv_pcpu_input_arg = alloc_percpu(void *); > > + > > + if (hyperv_pcpu_input_arg == NULL) > > + return; > > Huch. When that allocation fails, you return and ignore the rest of the > function which has been there before. Weird decision. I should have explained this. Failure of this allocation means that we would not have the per-cpu hypercall input page which in turn would mean that we would not be able to invoke any hypercalls. Regards, K. Y
RE: [PATCH 1/5] X86: Hyper-V: Enlighten APIC access
> -Original Message- > From: Thomas Gleixner <t...@linutronix.de> > Sent: Thursday, April 26, 2018 2:49 PM > To: KY Srinivasan <k...@microsoft.com> > Cc: x...@kernel.org; gre...@linuxfoundation.org; linux- > ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de; > a...@canonical.com; jasow...@redhat.com; h...@zytor.com; Stephen > Hemminger <sthem...@microsoft.com>; Michael Kelley (EOSG) > <michael.h.kel...@microsoft.com>; vkuzn...@redhat.com > Subject: Re: [PATCH 1/5] X86: Hyper-V: Enlighten APIC access > > On Wed, 25 Apr 2018, k...@linuxonhyperv.com wrote: > > --- /dev/null > > +++ b/arch/x86/hyperv/hv_apic.c > > @@ -0,0 +1,98 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > Thanks for putting the license identifier in. > > > + > > +/* > > + * Hyper-V specific APIC code. > > + * > > + * Copyright (C) 2018, Microsoft, Inc. > > + * > > + * Author : K. Y. Srinivasan <k...@microsoft.com> > > But can you please check with your lawyers whether you can avoid the > pointless boilerplate? The SPDX identifier should cover it. I will consult with MSFT legal on this. > > > + * This program is free software; you can redistribute it and/or modify it > > + * under the terms of the GNU General Public License version 2 as > published > > + * by the Free Software Foundation. > > + * > > + * This program is distributed in the hope that it will be useful, but > > + * WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD > TITLE or > > + * NON INFRINGEMENT. See the GNU General Public License for more > > + * details. > > + * > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > We usually order the includes > > #include > ... > #include > > #include > #include > > > > -void hyperv_init(void); > > +void __init hyperv_init(void); > > void hyperv_setup_mmu_ops(void); > > void hyper_alloc_mmu(void); > > void hyperv_report_panic(struct pt_regs *regs, long err); > > @@ -269,14 +269,16 @@ void hyperv_reenlightenment_intr(struct pt_regs > *regs); > > void set_hv_tscchange_cb(void (*cb)(void)); > > void clear_hv_tscchange_cb(void); > > void hyperv_stop_tsc_emulation(void); > > +void hv_apic_init(void); > > #else /* CONFIG_HYPERV */ > > -static inline void hyperv_init(void) {} > > +static __init inline void hyperv_init(void) {} > > The __init on the empty inline function is pointless. > > Other than the few nits. This looks well done! Thanks Thomas. I will address all the issues you have brought up in the next version. Regards, K. Y
RE: [PATCH 1/5] X86: Hyper-V: Enlighten APIC access
> -Original Message- > From: Thomas Gleixner > Sent: Thursday, April 26, 2018 2:49 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; h...@zytor.com; Stephen > Hemminger ; Michael Kelley (EOSG) > ; vkuzn...@redhat.com > Subject: Re: [PATCH 1/5] X86: Hyper-V: Enlighten APIC access > > On Wed, 25 Apr 2018, k...@linuxonhyperv.com wrote: > > --- /dev/null > > +++ b/arch/x86/hyperv/hv_apic.c > > @@ -0,0 +1,98 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > Thanks for putting the license identifier in. > > > + > > +/* > > + * Hyper-V specific APIC code. > > + * > > + * Copyright (C) 2018, Microsoft, Inc. > > + * > > + * Author : K. Y. Srinivasan > > But can you please check with your lawyers whether you can avoid the > pointless boilerplate? The SPDX identifier should cover it. I will consult with MSFT legal on this. > > > + * This program is free software; you can redistribute it and/or modify it > > + * under the terms of the GNU General Public License version 2 as > published > > + * by the Free Software Foundation. > > + * > > + * This program is distributed in the hope that it will be useful, but > > + * WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD > TITLE or > > + * NON INFRINGEMENT. See the GNU General Public License for more > > + * details. > > + * > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > We usually order the includes > > #include > ... > #include > > #include > #include > > > > -void hyperv_init(void); > > +void __init hyperv_init(void); > > void hyperv_setup_mmu_ops(void); > > void hyper_alloc_mmu(void); > > void hyperv_report_panic(struct pt_regs *regs, long err); > > @@ -269,14 +269,16 @@ void hyperv_reenlightenment_intr(struct pt_regs > *regs); > > void set_hv_tscchange_cb(void (*cb)(void)); > > void clear_hv_tscchange_cb(void); > > void hyperv_stop_tsc_emulation(void); > > +void hv_apic_init(void); > > #else /* CONFIG_HYPERV */ > > -static inline void hyperv_init(void) {} > > +static __init inline void hyperv_init(void) {} > > The __init on the empty inline function is pointless. > > Other than the few nits. This looks well done! Thanks Thomas. I will address all the issues you have brought up in the next version. Regards, K. Y
RE: [PATCH] scsi: devinfo: Add Microsoft iSCSI target to 1024 sector blacklist
+Matt > -Original Message- > From: Ross Lagerwall <ross.lagerw...@citrix.com> > Sent: Thursday, April 5, 2018 9:58 AM > To: Long Li <lon...@microsoft.com>; Martin K. Petersen > <martin.peter...@oracle.com>; KY Srinivasan <k...@microsoft.com> > Cc: James E.J. Bottomley <j...@linux.vnet.ibm.com>; linux- > s...@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH] scsi: devinfo: Add Microsoft iSCSI target to 1024 sector > blacklist > > On 03/28/2018 11:33 PM, Long Li wrote: > >> Subject: Re: [PATCH] scsi: devinfo: Add Microsoft iSCSI target to 1024 > sector > >> blacklist > >> > >> > >> Long, KY: Please confirm. > >> > >>> The Windows Server 2016 iSCSI target doesn't work with the Linux > >>> kernel initiator since the kernel started sending larger requests by > >>> default, nor does it implement the block limits VPD page. Apply the > >>> sector limit workaround for these targets. > >>> > >>> Signed-off-by: Ross Lagerwall <ross.lagerw...@citrix.com> > >>> --- > >>> drivers/scsi/scsi_devinfo.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c > >>> index f3b1172..5cb748a 100644 > >>> --- a/drivers/scsi/scsi_devinfo.c > >>> +++ b/drivers/scsi/scsi_devinfo.c > >>> @@ -213,7 +213,7 @@ static struct { > >>> {"Medion", "Flash XL MMC/SD", "2.6D", BLIST_FORCELUN}, > >>> {"MegaRAID", "LD", NULL, BLIST_FORCELUN}, > >>> {"MICROP", "4110", NULL, BLIST_NOTQ}, > >>> - {"MSFT", "Virtual HD", NULL, BLIST_NO_RSOC}, > >>> + {"MSFT", "Virtual HD", NULL, BLIST_MAX_1024 | BLIST_NO_RSOC}, > > > > Ross, > > > > What about storage_channel_properties.max_transfer_bytes returned > from VSTOR_OPERATION_QUERY_PROPERTIES (in storvsc_channel_init()) > > > > Does it return correctly the maximum transfer size for iSCSI? > > > > I presume you're referring to the Hyper-V virtual storage driver? This > has nothing to do with that module -- I don't even have it compiled in. > It's just simply the Linux kernel initiator connecting over plain > software iSCSI to a Windows Server 2016 iSCSI target. > > This is easy enough to reproduce. Just set up a Windows Server 2016 > target and try and use it from Linux. You get I/O errors as soon as you > try and format the disk. Adding Matt for visibility. Ross, we are fine with blacklisting for now. K. Y > > Cheers, > -- > Ross Lagerwall
RE: [PATCH] scsi: devinfo: Add Microsoft iSCSI target to 1024 sector blacklist
+Matt > -Original Message- > From: Ross Lagerwall > Sent: Thursday, April 5, 2018 9:58 AM > To: Long Li ; Martin K. Petersen > ; KY Srinivasan > Cc: James E.J. Bottomley ; linux- > s...@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH] scsi: devinfo: Add Microsoft iSCSI target to 1024 sector > blacklist > > On 03/28/2018 11:33 PM, Long Li wrote: > >> Subject: Re: [PATCH] scsi: devinfo: Add Microsoft iSCSI target to 1024 > sector > >> blacklist > >> > >> > >> Long, KY: Please confirm. > >> > >>> The Windows Server 2016 iSCSI target doesn't work with the Linux > >>> kernel initiator since the kernel started sending larger requests by > >>> default, nor does it implement the block limits VPD page. Apply the > >>> sector limit workaround for these targets. > >>> > >>> Signed-off-by: Ross Lagerwall > >>> --- > >>> drivers/scsi/scsi_devinfo.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c > >>> index f3b1172..5cb748a 100644 > >>> --- a/drivers/scsi/scsi_devinfo.c > >>> +++ b/drivers/scsi/scsi_devinfo.c > >>> @@ -213,7 +213,7 @@ static struct { > >>> {"Medion", "Flash XL MMC/SD", "2.6D", BLIST_FORCELUN}, > >>> {"MegaRAID", "LD", NULL, BLIST_FORCELUN}, > >>> {"MICROP", "4110", NULL, BLIST_NOTQ}, > >>> - {"MSFT", "Virtual HD", NULL, BLIST_NO_RSOC}, > >>> + {"MSFT", "Virtual HD", NULL, BLIST_MAX_1024 | BLIST_NO_RSOC}, > > > > Ross, > > > > What about storage_channel_properties.max_transfer_bytes returned > from VSTOR_OPERATION_QUERY_PROPERTIES (in storvsc_channel_init()) > > > > Does it return correctly the maximum transfer size for iSCSI? > > > > I presume you're referring to the Hyper-V virtual storage driver? This > has nothing to do with that module -- I don't even have it compiled in. > It's just simply the Linux kernel initiator connecting over plain > software iSCSI to a Windows Server 2016 iSCSI target. > > This is easy enough to reproduce. Just set up a Windows Server 2016 > target and try and use it from Linux. You get I/O errors as soon as you > try and format the disk. Adding Matt for visibility. Ross, we are fine with blacklisting for now. K. Y > > Cheers, > -- > Ross Lagerwall
RE: [PATCH 2/3] Netvsc: Use the vmbus functiton to calculate ring buffer percentage
> -Original Message- > From: Haiyang Zhang > Sent: Friday, March 23, 2018 8:01 AM > To: Long Li <lon...@linuxonhyperv.com>; KY Srinivasan > <k...@microsoft.com>; Stephen Hemminger <sthem...@microsoft.com>; > James E . J . Bottomley <jbottom...@odin.com>; Martin K . Petersen > <martin.peter...@oracle.com>; de...@linuxdriverproject.org; linux- > s...@vger.kernel.org; linux-kernel@vger.kernel.org > Cc: Long Li <lon...@microsoft.com> > Subject: RE: [PATCH 2/3] Netvsc: Use the vmbus functiton to calculate ring > buffer percentage > > > > > -Original Message- > > From: Long Li <lon...@linuxonhyperv.com> > > Sent: Thursday, March 22, 2018 8:16 PM > > To: KY Srinivasan <k...@microsoft.com>; Haiyang Zhang > > <haiya...@microsoft.com>; Stephen Hemminger > <sthem...@microsoft.com>; > > James E . J . Bottomley <jbottom...@odin.com>; Martin K . Petersen > > <martin.peter...@oracle.com>; de...@linuxdriverproject.org; linux- > > s...@vger.kernel.org; linux-kernel@vger.kernel.org > > Cc: Long Li <lon...@microsoft.com> > > Subject: [PATCH 2/3] Netvsc: Use the vmbus functiton to calculate ring > buffer > > percentage > > > > From: Long Li <lon...@microsoft.com> > > > > In Vmbus, we have defined a function to calculate available ring buffer > > percentage to write. > > > > Use that function and remove duplicate netvsc code. > > > > Signed-off-by: Long Li <lon...@microsoft.com> > > --- > > drivers/net/hyperv/netvsc.c | 17 +++-- > > drivers/net/hyperv/netvsc_drv.c | 3 --- > > 2 files changed, 3 insertions(+), 17 deletions(-) Why is the patch being sent to the scsi list and not to the network mailing list and Dave Miller. K. Y > > > > diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c > index > > 0265d703eb03..8af0069e4d8c 100644 > > --- a/drivers/net/hyperv/netvsc.c > > +++ b/drivers/net/hyperv/netvsc.c > > @@ -31,7 +31,6 @@ > > #include > > #include > > #include > > -#include > > > > #include > > > > @@ -590,17 +589,6 @@ void netvsc_device_remove(struct hv_device > *device) > > #define RING_AVAIL_PERCENT_HIWATER 20 #define > > RING_AVAIL_PERCENT_LOWATER 10 > > > > -/* > > - * Get the percentage of available bytes to write in the ring. > > - * The return value is in range from 0 to 100. > > - */ > > -static u32 hv_ringbuf_avail_percent(const struct hv_ring_buffer_info > > *ring_info) -{ > > - u32 avail_write = hv_get_bytes_to_write(ring_info); > > - > > - return reciprocal_divide(avail_write * 100, netvsc_ring_reciprocal); > > -} > > - > > static inline void netvsc_free_send_slot(struct netvsc_device > *net_device, > > u32 index) > > { > > @@ -649,7 +637,8 @@ static void netvsc_send_tx_complete(struct > > netvsc_device *net_device, > > wake_up(_device->wait_drain); > > > > if (netif_tx_queue_stopped(netdev_get_tx_queue(ndev, q_idx)) > && > > - (hv_ringbuf_avail_percent(>outbound) > > > RING_AVAIL_PERCENT_HIWATER || > > + (hv_get_avail_to_write_percent(>outbound) > > > +RING_AVAIL_PERCENT_HIWATER || > > queue_sends < 1)) { > > netif_tx_wake_queue(netdev_get_tx_queue(ndev, q_idx)); > > ndev_ctx->eth_stats.wake_queue++; > > @@ -757,7 +746,7 @@ static inline int netvsc_send_pkt( > > struct netdev_queue *txq = netdev_get_tx_queue(ndev, packet- > >q_idx); > > u64 req_id; > > int ret; > > - u32 ring_avail = hv_ringbuf_avail_percent(_channel- > >outbound); > > + u32 ring_avail = > > +hv_get_avail_to_write_percent(_channel->outbound); > > > > nvmsg.hdr.msg_type = NVSP_MSG1_TYPE_SEND_RNDIS_PKT; > > if (skb) > > diff --git a/drivers/net/hyperv/netvsc_drv.c > b/drivers/net/hyperv/netvsc_drv.c > > index faea0be18924..b0b1c2fd2b7b 100644 > > --- a/drivers/net/hyperv/netvsc_drv.c > > +++ b/drivers/net/hyperv/netvsc_drv.c > > @@ -35,7 +35,6 @@ > > #include > > #include > > #include > > -#include > > > > #include > > #include > > @@ -55,7 +54,6 @@ static unsigned int ring_size __ro_after_init = 128; > > module_param(ring_size, uint, S_IRUGO); > MODULE_PARM_DESC(ring_size, > > "Ring buffer size (# of pages)"); unsigned int netvsc_ring_bytes > __ro_after_init; > > -struct reciprocal_value netvsc_ring_reciprocal __ro_after_init; > > > > static const u32 default_msg = NETIF_MSG_DRV | NETIF_MSG_PROBE | > > NETIF_MSG_LINK | NETIF_MSG_IFUP | > > @@ -2186,7 +2184,6 @@ static int __init netvsc_drv_init(void) > > ring_size); > > } > > netvsc_ring_bytes = ring_size * PAGE_SIZE; > > - netvsc_ring_reciprocal = reciprocal_value(netvsc_ring_bytes); > > > > ret = vmbus_driver_register(_drv); > > if (ret) > > -- > > > Please also remove netvsc_ring_reciprocal from hyperv_net.h > Thanks. > > Reviewed-by: Haiyang Zhang <haiya...@microsoft.com>
RE: [PATCH 2/3] Netvsc: Use the vmbus functiton to calculate ring buffer percentage
> -Original Message- > From: Haiyang Zhang > Sent: Friday, March 23, 2018 8:01 AM > To: Long Li ; KY Srinivasan > ; Stephen Hemminger ; > James E . J . Bottomley ; Martin K . Petersen > ; de...@linuxdriverproject.org; linux- > s...@vger.kernel.org; linux-kernel@vger.kernel.org > Cc: Long Li > Subject: RE: [PATCH 2/3] Netvsc: Use the vmbus functiton to calculate ring > buffer percentage > > > > > -Original Message- > > From: Long Li > > Sent: Thursday, March 22, 2018 8:16 PM > > To: KY Srinivasan ; Haiyang Zhang > > ; Stephen Hemminger > ; > > James E . J . Bottomley ; Martin K . Petersen > > ; de...@linuxdriverproject.org; linux- > > s...@vger.kernel.org; linux-kernel@vger.kernel.org > > Cc: Long Li > > Subject: [PATCH 2/3] Netvsc: Use the vmbus functiton to calculate ring > buffer > > percentage > > > > From: Long Li > > > > In Vmbus, we have defined a function to calculate available ring buffer > > percentage to write. > > > > Use that function and remove duplicate netvsc code. > > > > Signed-off-by: Long Li > > --- > > drivers/net/hyperv/netvsc.c | 17 +++-- > > drivers/net/hyperv/netvsc_drv.c | 3 --- > > 2 files changed, 3 insertions(+), 17 deletions(-) Why is the patch being sent to the scsi list and not to the network mailing list and Dave Miller. K. Y > > > > diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c > index > > 0265d703eb03..8af0069e4d8c 100644 > > --- a/drivers/net/hyperv/netvsc.c > > +++ b/drivers/net/hyperv/netvsc.c > > @@ -31,7 +31,6 @@ > > #include > > #include > > #include > > -#include > > > > #include > > > > @@ -590,17 +589,6 @@ void netvsc_device_remove(struct hv_device > *device) > > #define RING_AVAIL_PERCENT_HIWATER 20 #define > > RING_AVAIL_PERCENT_LOWATER 10 > > > > -/* > > - * Get the percentage of available bytes to write in the ring. > > - * The return value is in range from 0 to 100. > > - */ > > -static u32 hv_ringbuf_avail_percent(const struct hv_ring_buffer_info > > *ring_info) -{ > > - u32 avail_write = hv_get_bytes_to_write(ring_info); > > - > > - return reciprocal_divide(avail_write * 100, netvsc_ring_reciprocal); > > -} > > - > > static inline void netvsc_free_send_slot(struct netvsc_device > *net_device, > > u32 index) > > { > > @@ -649,7 +637,8 @@ static void netvsc_send_tx_complete(struct > > netvsc_device *net_device, > > wake_up(_device->wait_drain); > > > > if (netif_tx_queue_stopped(netdev_get_tx_queue(ndev, q_idx)) > && > > - (hv_ringbuf_avail_percent(>outbound) > > > RING_AVAIL_PERCENT_HIWATER || > > + (hv_get_avail_to_write_percent(>outbound) > > > +RING_AVAIL_PERCENT_HIWATER || > > queue_sends < 1)) { > > netif_tx_wake_queue(netdev_get_tx_queue(ndev, q_idx)); > > ndev_ctx->eth_stats.wake_queue++; > > @@ -757,7 +746,7 @@ static inline int netvsc_send_pkt( > > struct netdev_queue *txq = netdev_get_tx_queue(ndev, packet- > >q_idx); > > u64 req_id; > > int ret; > > - u32 ring_avail = hv_ringbuf_avail_percent(_channel- > >outbound); > > + u32 ring_avail = > > +hv_get_avail_to_write_percent(_channel->outbound); > > > > nvmsg.hdr.msg_type = NVSP_MSG1_TYPE_SEND_RNDIS_PKT; > > if (skb) > > diff --git a/drivers/net/hyperv/netvsc_drv.c > b/drivers/net/hyperv/netvsc_drv.c > > index faea0be18924..b0b1c2fd2b7b 100644 > > --- a/drivers/net/hyperv/netvsc_drv.c > > +++ b/drivers/net/hyperv/netvsc_drv.c > > @@ -35,7 +35,6 @@ > > #include > > #include > > #include > > -#include > > > > #include > > #include > > @@ -55,7 +54,6 @@ static unsigned int ring_size __ro_after_init = 128; > > module_param(ring_size, uint, S_IRUGO); > MODULE_PARM_DESC(ring_size, > > "Ring buffer size (# of pages)"); unsigned int netvsc_ring_bytes > __ro_after_init; > > -struct reciprocal_value netvsc_ring_reciprocal __ro_after_init; > > > > static const u32 default_msg = NETIF_MSG_DRV | NETIF_MSG_PROBE | > > NETIF_MSG_LINK | NETIF_MSG_IFUP | > > @@ -2186,7 +2184,6 @@ static int __init netvsc_drv_init(void) > > ring_size); > > } > > netvsc_ring_bytes = ring_size * PAGE_SIZE; > > - netvsc_ring_reciprocal = reciprocal_value(netvsc_ring_bytes); > > > > ret = vmbus_driver_register(_drv); > > if (ret) > > -- > > > Please also remove netvsc_ring_reciprocal from hyperv_net.h > Thanks. > > Reviewed-by: Haiyang Zhang
RE: [PATCH] vmbus: use put_device() if device_register fail
> -Original Message- > From: Michael Kelley (EOSG) > Sent: Sunday, March 18, 2018 8:40 PM > To: KY Srinivasan <k...@microsoft.com>; Arvind Yadav > <arvind.yadav...@gmail.com>; Stephen Hemminger > <sthem...@microsoft.com>; Haiyang Zhang <haiya...@microsoft.com> > Cc: de...@linuxdriverproject.org; linux-kernel@vger.kernel.org > Subject: RE: [PATCH] vmbus: use put_device() if device_register fail > > > -Original Message- > > From: linux-kernel-ow...@vger.kernel.org ow...@vger.kernel.org> On Behalf > > Of KY Srinivasan > > Sent: Sunday, March 18, 2018 8:02 PM > > To: Arvind Yadav <arvind.yadav...@gmail.com>; Stephen Hemminger > > <sthem...@microsoft.com>; Haiyang Zhang <haiya...@microsoft.com> > > Cc: de...@linuxdriverproject.org; linux-kernel@vger.kernel.org > > Subject: RE: [PATCH] vmbus: use put_device() if device_register fail > > > > > -Original Message- > > > From: devel <driverdev-devel-boun...@linuxdriverproject.org> On > Behalf > > > Of Arvind Yadav > > > Sent: Saturday, March 17, 2018 11:48 PM > > > To: Stephen Hemminger <sthem...@microsoft.com>; Haiyang Zhang > > > <haiya...@microsoft.com> > > > Cc: de...@linuxdriverproject.org; linux-kernel@vger.kernel.org > > > Subject: [PATCH] vmbus: use put_device() if device_register fail > > > > > > if device_register() returned an error. Always use put_device() > > > to give up the reference initialized. > > > > > > Signed-off-by: Arvind Yadav <arvind.yadav...@gmail.com> > > > --- > > > drivers/hv/vmbus_drv.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c > > > index bc65c4d..25da2f3 100644 > > > --- a/drivers/hv/vmbus_drv.c > > > +++ b/drivers/hv/vmbus_drv.c > > > @@ -1358,6 +1358,7 @@ int vmbus_device_register(struct hv_device > > > *child_device_obj) > > > ret = device_register(_device_obj->device); > > > if (ret) { > > > pr_err("Unable to register child device\n"); > > > + put_device(_device_obj->device); > > > > If the registration failed; we would not have acquired the reference on the > device and so > > we should not be dropping the reference in the failure path. > > Looking at the code for device_register() and its constituent parts > device_initialize() and device_add(), Arvind is correct. device_initialize() > creates the object with a reference count of 1. device_add() does not > decrement that reference count or free the object, even if it fails. The > comments for device_register() call this out as well. Yes; agreed. I will take this patch. K. Y > > Michael > > > > > K. Y > > > return ret; > > > } > > > > > > -- > > > 2.7.4
RE: [PATCH] vmbus: use put_device() if device_register fail
> -Original Message- > From: Michael Kelley (EOSG) > Sent: Sunday, March 18, 2018 8:40 PM > To: KY Srinivasan ; Arvind Yadav > ; Stephen Hemminger > ; Haiyang Zhang > Cc: de...@linuxdriverproject.org; linux-kernel@vger.kernel.org > Subject: RE: [PATCH] vmbus: use put_device() if device_register fail > > > -Original Message- > > From: linux-kernel-ow...@vger.kernel.org ow...@vger.kernel.org> On Behalf > > Of KY Srinivasan > > Sent: Sunday, March 18, 2018 8:02 PM > > To: Arvind Yadav ; Stephen Hemminger > > ; Haiyang Zhang > > Cc: de...@linuxdriverproject.org; linux-kernel@vger.kernel.org > > Subject: RE: [PATCH] vmbus: use put_device() if device_register fail > > > > > -Original Message- > > > From: devel On > Behalf > > > Of Arvind Yadav > > > Sent: Saturday, March 17, 2018 11:48 PM > > > To: Stephen Hemminger ; Haiyang Zhang > > > > > > Cc: de...@linuxdriverproject.org; linux-kernel@vger.kernel.org > > > Subject: [PATCH] vmbus: use put_device() if device_register fail > > > > > > if device_register() returned an error. Always use put_device() > > > to give up the reference initialized. > > > > > > Signed-off-by: Arvind Yadav > > > --- > > > drivers/hv/vmbus_drv.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c > > > index bc65c4d..25da2f3 100644 > > > --- a/drivers/hv/vmbus_drv.c > > > +++ b/drivers/hv/vmbus_drv.c > > > @@ -1358,6 +1358,7 @@ int vmbus_device_register(struct hv_device > > > *child_device_obj) > > > ret = device_register(_device_obj->device); > > > if (ret) { > > > pr_err("Unable to register child device\n"); > > > + put_device(_device_obj->device); > > > > If the registration failed; we would not have acquired the reference on the > device and so > > we should not be dropping the reference in the failure path. > > Looking at the code for device_register() and its constituent parts > device_initialize() and device_add(), Arvind is correct. device_initialize() > creates the object with a reference count of 1. device_add() does not > decrement that reference count or free the object, even if it fails. The > comments for device_register() call this out as well. Yes; agreed. I will take this patch. K. Y > > Michael > > > > > K. Y > > > return ret; > > > } > > > > > > -- > > > 2.7.4
RE: [PATCH] vmbus: use put_device() if device_register fail
> -Original Message- > From: develOn Behalf > Of Arvind Yadav > Sent: Saturday, March 17, 2018 11:48 PM > To: Stephen Hemminger ; Haiyang Zhang > > Cc: de...@linuxdriverproject.org; linux-kernel@vger.kernel.org > Subject: [PATCH] vmbus: use put_device() if device_register fail > > if device_register() returned an error. Always use put_device() > to give up the reference initialized. > > Signed-off-by: Arvind Yadav > --- > drivers/hv/vmbus_drv.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c > index bc65c4d..25da2f3 100644 > --- a/drivers/hv/vmbus_drv.c > +++ b/drivers/hv/vmbus_drv.c > @@ -1358,6 +1358,7 @@ int vmbus_device_register(struct hv_device > *child_device_obj) > ret = device_register(_device_obj->device); > if (ret) { > pr_err("Unable to register child device\n"); > + put_device(_device_obj->device); If the registration failed; we would not have acquired the reference on the device and so we should not be dropping the reference in the failure path. K. Y > return ret; > } > > -- > 2.7.4 > > ___ > devel mailing list > de...@linuxdriverproject.org > https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdriverd > ev.linuxdriverproject.org%2Fmailman%2Flistinfo%2Fdriverdev- > devel=04%7C01%7Ckys%40microsoft.com%7Caf95bfab917f4e1fa4b008 > d58c9c3b72%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63656952 > 5011478334%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj > oiV2luMzIiLCJBTiI6Ik1haWwifQ%3D%3D%7C- > 1=%2FEb%2FMTY2SMh8sY1tar2c8Om2uKPUZAXIUQkrG0q07CA%3D > eserved=0
RE: [PATCH] vmbus: use put_device() if device_register fail
> -Original Message- > From: devel On Behalf > Of Arvind Yadav > Sent: Saturday, March 17, 2018 11:48 PM > To: Stephen Hemminger ; Haiyang Zhang > > Cc: de...@linuxdriverproject.org; linux-kernel@vger.kernel.org > Subject: [PATCH] vmbus: use put_device() if device_register fail > > if device_register() returned an error. Always use put_device() > to give up the reference initialized. > > Signed-off-by: Arvind Yadav > --- > drivers/hv/vmbus_drv.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c > index bc65c4d..25da2f3 100644 > --- a/drivers/hv/vmbus_drv.c > +++ b/drivers/hv/vmbus_drv.c > @@ -1358,6 +1358,7 @@ int vmbus_device_register(struct hv_device > *child_device_obj) > ret = device_register(_device_obj->device); > if (ret) { > pr_err("Unable to register child device\n"); > + put_device(_device_obj->device); If the registration failed; we would not have acquired the reference on the device and so we should not be dropping the reference in the failure path. K. Y > return ret; > } > > -- > 2.7.4 > > ___ > devel mailing list > de...@linuxdriverproject.org > https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdriverd > ev.linuxdriverproject.org%2Fmailman%2Flistinfo%2Fdriverdev- > devel=04%7C01%7Ckys%40microsoft.com%7Caf95bfab917f4e1fa4b008 > d58c9c3b72%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63656952 > 5011478334%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj > oiV2luMzIiLCJBTiI6Ik1haWwifQ%3D%3D%7C- > 1=%2FEb%2FMTY2SMh8sY1tar2c8Om2uKPUZAXIUQkrG0q07CA%3D > eserved=0
RE: [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in hv_pci_onchannelcallback
> -Original Message- > From: Jia-Ju Bai <baijiaju1...@gmail.com> > Sent: Sunday, March 18, 2018 7:53 AM > To: KY Srinivasan <k...@microsoft.com>; Haiyang Zhang > <haiya...@microsoft.com>; Stephen Hemminger > <sthem...@microsoft.com>; bhelg...@google.com > Cc: de...@linuxdriverproject.org; linux-...@vger.kernel.org; linux- > ker...@vger.kernel.org; Jia-Ju Bai <baijiaju1...@gmail.com> > Subject: [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with > GFP_KERNEL in hv_pci_onchannelcallback > > hv_pci_onchannelcallback() is not called in atomic context. > > The call chain ending up at hv_pci_onchannelcallback() is: > [1] hv_pci_onchannelcallback() <- hv_pci_probe() > hv_pci_probe() is only set as ".probe" in hv_driver > structure "hv_pci_drv". This function is setup as the function to handle interrupts on the channel. Hence the need for GFP_ATOMIC. K. Y > > Despite never getting called from atomic context, > hv_pci_onchannelcallback() calls kmalloc with GFP_ATOMIC, > which waits busily for allocation. > GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL > to avoid busy waiting. > > This is found by a static analysis tool named DCNS written by myself. > > Signed-off-by: Jia-Ju Bai <baijiaju1...@gmail.com> > --- > drivers/pci/host/pci-hyperv.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c > index 0fe3ea1..c5c8a99 100644 > --- a/drivers/pci/host/pci-hyperv.c > +++ b/drivers/pci/host/pci-hyperv.c > @@ -1887,7 +1887,7 @@ static void hv_pci_onchannelcallback(void > *context) > struct pci_dev_incoming *dev_message; > struct hv_pci_dev *hpdev; > > - buffer = kmalloc(bufferlen, GFP_ATOMIC); > + buffer = kmalloc(bufferlen, GFP_KERNEL); > if (!buffer) > return; > > @@ -1899,7 +1899,7 @@ static void hv_pci_onchannelcallback(void > *context) > kfree(buffer); > /* Handle large packet */ > bufferlen = bytes_recvd; > - buffer = kmalloc(bytes_recvd, GFP_ATOMIC); > + buffer = kmalloc(bytes_recvd, GFP_KERNEL); > if (!buffer) > return; > continue; > -- > 1.9.1
RE: [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in hv_pci_onchannelcallback
> -Original Message- > From: Jia-Ju Bai > Sent: Sunday, March 18, 2018 7:53 AM > To: KY Srinivasan ; Haiyang Zhang > ; Stephen Hemminger > ; bhelg...@google.com > Cc: de...@linuxdriverproject.org; linux-...@vger.kernel.org; linux- > ker...@vger.kernel.org; Jia-Ju Bai > Subject: [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with > GFP_KERNEL in hv_pci_onchannelcallback > > hv_pci_onchannelcallback() is not called in atomic context. > > The call chain ending up at hv_pci_onchannelcallback() is: > [1] hv_pci_onchannelcallback() <- hv_pci_probe() > hv_pci_probe() is only set as ".probe" in hv_driver > structure "hv_pci_drv". This function is setup as the function to handle interrupts on the channel. Hence the need for GFP_ATOMIC. K. Y > > Despite never getting called from atomic context, > hv_pci_onchannelcallback() calls kmalloc with GFP_ATOMIC, > which waits busily for allocation. > GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL > to avoid busy waiting. > > This is found by a static analysis tool named DCNS written by myself. > > Signed-off-by: Jia-Ju Bai > --- > drivers/pci/host/pci-hyperv.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c > index 0fe3ea1..c5c8a99 100644 > --- a/drivers/pci/host/pci-hyperv.c > +++ b/drivers/pci/host/pci-hyperv.c > @@ -1887,7 +1887,7 @@ static void hv_pci_onchannelcallback(void > *context) > struct pci_dev_incoming *dev_message; > struct hv_pci_dev *hpdev; > > - buffer = kmalloc(bufferlen, GFP_ATOMIC); > + buffer = kmalloc(bufferlen, GFP_KERNEL); > if (!buffer) > return; > > @@ -1899,7 +1899,7 @@ static void hv_pci_onchannelcallback(void > *context) > kfree(buffer); > /* Handle large packet */ > bufferlen = bytes_recvd; > - buffer = kmalloc(bytes_recvd, GFP_ATOMIC); > + buffer = kmalloc(bytes_recvd, GFP_KERNEL); > if (!buffer) > return; > continue; > -- > 1.9.1
RE: [PATCH] X86/UAPI: Use __u64 instead of u64 in hyperv.h
> -Original Message- > From: KarimAllah Ahmed <karah...@amazon.de> > Sent: Monday, February 19, 2018 11:40 PM > To: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org; > x...@kernel.org > Cc: KarimAllah Ahmed <karah...@amazon.de>; KY Srinivasan > <k...@microsoft.com>; Haiyang Zhang <haiya...@microsoft.com>; Stephen > Hemminger <sthem...@microsoft.com>; Thomas Gleixner > <t...@linutronix.de>; Ingo Molnar <mi...@redhat.com>; H . Peter Anvin > <h...@zytor.com> > Subject: [PATCH] X86/UAPI: Use __u64 instead of u64 in hyperv.h > > ... since u64 has a hidden header dependency that was not there before > using it (i.e. it breaks our VMM build). Also, __u64 is the right way to > expose the data type through UAPI. > > Fixes: 93286261 ("x86/hyperv: Reenlightenment notifications support") > Signed-off-by: KarimAllah Ahmed <karah...@amazon.de> > Cc: K. Y. Srinivasan <k...@microsoft.com> > Cc: Haiyang Zhang <haiya...@microsoft.com> > Cc: Stephen Hemminger <sthem...@microsoft.com> > Cc: Thomas Gleixner <t...@linutronix.de> > Cc: Ingo Molnar <mi...@redhat.com> > Cc: H. Peter Anvin <h...@zytor.com> > Cc: x...@kernel.org > Cc: de...@linuxdriverproject.org > Cc: linux-kernel@vger.kernel.org Acked-by: K. Y. Srinivasan <k...@microsoft.com> > --- > arch/x86/include/uapi/asm/hyperv.h | 18 +- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/arch/x86/include/uapi/asm/hyperv.h > b/arch/x86/include/uapi/asm/hyperv.h > index 197c2e6..0994143 100644 > --- a/arch/x86/include/uapi/asm/hyperv.h > +++ b/arch/x86/include/uapi/asm/hyperv.h > @@ -241,24 +241,24 @@ > #define HV_X64_MSR_REENLIGHTENMENT_CONTROL 0x4106 > > struct hv_reenlightenment_control { > - u64 vector:8; > - u64 reserved1:8; > - u64 enabled:1; > - u64 reserved2:15; > - u64 target_vp:32; > + __u64 vector:8; > + __u64 reserved1:8; > + __u64 enabled:1; > + __u64 reserved2:15; > + __u64 target_vp:32; > }; > > #define HV_X64_MSR_TSC_EMULATION_CONTROL 0x4107 > #define HV_X64_MSR_TSC_EMULATION_STATUS 0x4108 > > struct hv_tsc_emulation_control { > - u64 enabled:1; > - u64 reserved:63; > + __u64 enabled:1; > + __u64 reserved:63; > }; > > struct hv_tsc_emulation_status { > - u64 inprogress:1; > - u64 reserved:63; > + __u64 inprogress:1; > + __u64 reserved:63; > }; > > #define HV_X64_MSR_HYPERCALL_ENABLE 0x0001 > -- > 2.7.4
RE: [PATCH] X86/UAPI: Use __u64 instead of u64 in hyperv.h
> -Original Message- > From: KarimAllah Ahmed > Sent: Monday, February 19, 2018 11:40 PM > To: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org; > x...@kernel.org > Cc: KarimAllah Ahmed ; KY Srinivasan > ; Haiyang Zhang ; Stephen > Hemminger ; Thomas Gleixner > ; Ingo Molnar ; H . Peter Anvin > > Subject: [PATCH] X86/UAPI: Use __u64 instead of u64 in hyperv.h > > ... since u64 has a hidden header dependency that was not there before > using it (i.e. it breaks our VMM build). Also, __u64 is the right way to > expose the data type through UAPI. > > Fixes: 93286261 ("x86/hyperv: Reenlightenment notifications support") > Signed-off-by: KarimAllah Ahmed > Cc: K. Y. Srinivasan > Cc: Haiyang Zhang > Cc: Stephen Hemminger > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: H. Peter Anvin > Cc: x...@kernel.org > Cc: de...@linuxdriverproject.org > Cc: linux-kernel@vger.kernel.org Acked-by: K. Y. Srinivasan > --- > arch/x86/include/uapi/asm/hyperv.h | 18 +- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/arch/x86/include/uapi/asm/hyperv.h > b/arch/x86/include/uapi/asm/hyperv.h > index 197c2e6..0994143 100644 > --- a/arch/x86/include/uapi/asm/hyperv.h > +++ b/arch/x86/include/uapi/asm/hyperv.h > @@ -241,24 +241,24 @@ > #define HV_X64_MSR_REENLIGHTENMENT_CONTROL 0x4106 > > struct hv_reenlightenment_control { > - u64 vector:8; > - u64 reserved1:8; > - u64 enabled:1; > - u64 reserved2:15; > - u64 target_vp:32; > + __u64 vector:8; > + __u64 reserved1:8; > + __u64 enabled:1; > + __u64 reserved2:15; > + __u64 target_vp:32; > }; > > #define HV_X64_MSR_TSC_EMULATION_CONTROL 0x4107 > #define HV_X64_MSR_TSC_EMULATION_STATUS 0x4108 > > struct hv_tsc_emulation_control { > - u64 enabled:1; > - u64 reserved:63; > + __u64 enabled:1; > + __u64 reserved:63; > }; > > struct hv_tsc_emulation_status { > - u64 inprogress:1; > - u64 reserved:63; > + __u64 inprogress:1; > + __u64 reserved:63; > }; > > #define HV_X64_MSR_HYPERCALL_ENABLE 0x0001 > -- > 2.7.4
RE: [PATCH char-misc 1/1] Drivers: hv: vmbus: Fix ring buffer signaling
> -Original Message- > From: Michael Kelley [mailto:mhkel...@outlook.com] > Sent: Saturday, February 10, 2018 12:49 PM > To: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > vkuzn...@redhat.com; jasow...@redhat.com; > leann.ogasaw...@canonical.com; marcelo.ce...@canonical.com; Stephen > Hemminger <sthem...@microsoft.com>; KY Srinivasan > <k...@microsoft.com> > Cc: Michael Kelley (EOSG) <michael.h.kel...@microsoft.com> > Subject: [PATCH char-misc 1/1] Drivers: hv: vmbus: Fix ring buffer signaling > > Fix bugs in signaling the Hyper-V host when freeing space in the > host->guest ring buffer: > > 1. The interrupt_mask must not be used to determine whether to signal >on the host->guest ring buffer > 2. The ring buffer write_index must be read (via hv_get_bytes_to_write) >*after* pending_send_sz is read in order to avoid a race condition > 3. Comparisons with pending_send_sz must treat the "equals" case as >not-enough-space > 4. Don't signal if the pending_send_sz feature is not present. Older >versions of Hyper-V that don't implement this feature will poll. > > Fixes: 03bad714a161 ("vmbus: more host signalling avoidance") > Signed-off-by: Michael Kelley <mhkel...@outlook.com> > --- > drivers/hv/ring_buffer.c | 24 > 1 file changed, 16 insertions(+), 8 deletions(-) > > diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c > index 50e0714..b64be18 100644 > --- a/drivers/hv/ring_buffer.c > +++ b/drivers/hv/ring_buffer.c > @@ -423,7 +423,11 @@ struct vmpacket_descriptor * > void hv_pkt_iter_close(struct vmbus_channel *channel) > { > struct hv_ring_buffer_info *rbi = >inbound; > - u32 orig_write_sz = hv_get_bytes_to_write(rbi); > + u32 curr_write_sz; > + u32 delta = rbi->ring_buffer->read_index < rbi->priv_read_index ? > + (rbi->priv_read_index - rbi->ring_buffer- > >read_index) : > + (rbi->ring_datasize - rbi->ring_buffer->read_index + > + rbi->priv_read_index); > > /* >* Make sure all reads are done before we update the read index > since > @@ -446,27 +450,31 @@ void hv_pkt_iter_close(struct vmbus_channel > *channel) >*/ > virt_mb(); > > - /* If host has disabled notifications then skip */ > - if (rbi->ring_buffer->interrupt_mask) > - return; > - > if (rbi->ring_buffer->feature_bits.feat_pending_send_sz) { > u32 pending_sz = READ_ONCE(rbi->ring_buffer- > >pending_send_sz); > > /* > + * Ensure the read of write_index in > hv_get_bytes_to_write() > + * happens after the read of pending_send_sz. > + */ > + virt_rmb(); We can avoid the read barrier by making the initialization of curr_write_sz conditional on pending_send_sz being non-zero. Indeed you can make all the signaling code conditional on pending_send_sz being non-zero. > + curr_write_sz = hv_get_bytes_to_write(rbi); > + > + /* >* If there was space before we began iteration, >* then host was not blocked. Also handles case where >* pending_sz is zero then host has nothing pending >* and does not need to be signaled. >*/ > - if (orig_write_sz > pending_sz) > + if (curr_write_sz - delta > pending_sz) > return; > > /* If pending write will not fit, don't give false hope. */ > - if (hv_get_bytes_to_write(rbi) < pending_sz) > + if (curr_write_sz <= pending_sz) > return; > + > + vmbus_setevent(channel); > } > > - vmbus_setevent(channel); > } > EXPORT_SYMBOL_GPL(hv_pkt_iter_close); > -- > 1.8.3.1
RE: [PATCH char-misc 1/1] Drivers: hv: vmbus: Fix ring buffer signaling
> -Original Message- > From: Michael Kelley [mailto:mhkel...@outlook.com] > Sent: Saturday, February 10, 2018 12:49 PM > To: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > vkuzn...@redhat.com; jasow...@redhat.com; > leann.ogasaw...@canonical.com; marcelo.ce...@canonical.com; Stephen > Hemminger ; KY Srinivasan > > Cc: Michael Kelley (EOSG) > Subject: [PATCH char-misc 1/1] Drivers: hv: vmbus: Fix ring buffer signaling > > Fix bugs in signaling the Hyper-V host when freeing space in the > host->guest ring buffer: > > 1. The interrupt_mask must not be used to determine whether to signal >on the host->guest ring buffer > 2. The ring buffer write_index must be read (via hv_get_bytes_to_write) >*after* pending_send_sz is read in order to avoid a race condition > 3. Comparisons with pending_send_sz must treat the "equals" case as >not-enough-space > 4. Don't signal if the pending_send_sz feature is not present. Older >versions of Hyper-V that don't implement this feature will poll. > > Fixes: 03bad714a161 ("vmbus: more host signalling avoidance") > Signed-off-by: Michael Kelley > --- > drivers/hv/ring_buffer.c | 24 > 1 file changed, 16 insertions(+), 8 deletions(-) > > diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c > index 50e0714..b64be18 100644 > --- a/drivers/hv/ring_buffer.c > +++ b/drivers/hv/ring_buffer.c > @@ -423,7 +423,11 @@ struct vmpacket_descriptor * > void hv_pkt_iter_close(struct vmbus_channel *channel) > { > struct hv_ring_buffer_info *rbi = >inbound; > - u32 orig_write_sz = hv_get_bytes_to_write(rbi); > + u32 curr_write_sz; > + u32 delta = rbi->ring_buffer->read_index < rbi->priv_read_index ? > + (rbi->priv_read_index - rbi->ring_buffer- > >read_index) : > + (rbi->ring_datasize - rbi->ring_buffer->read_index + > + rbi->priv_read_index); > > /* >* Make sure all reads are done before we update the read index > since > @@ -446,27 +450,31 @@ void hv_pkt_iter_close(struct vmbus_channel > *channel) >*/ > virt_mb(); > > - /* If host has disabled notifications then skip */ > - if (rbi->ring_buffer->interrupt_mask) > - return; > - > if (rbi->ring_buffer->feature_bits.feat_pending_send_sz) { > u32 pending_sz = READ_ONCE(rbi->ring_buffer- > >pending_send_sz); > > /* > + * Ensure the read of write_index in > hv_get_bytes_to_write() > + * happens after the read of pending_send_sz. > + */ > + virt_rmb(); We can avoid the read barrier by making the initialization of curr_write_sz conditional on pending_send_sz being non-zero. Indeed you can make all the signaling code conditional on pending_send_sz being non-zero. > + curr_write_sz = hv_get_bytes_to_write(rbi); > + > + /* >* If there was space before we began iteration, >* then host was not blocked. Also handles case where >* pending_sz is zero then host has nothing pending >* and does not need to be signaled. >*/ > - if (orig_write_sz > pending_sz) > + if (curr_write_sz - delta > pending_sz) > return; > > /* If pending write will not fit, don't give false hope. */ > - if (hv_get_bytes_to_write(rbi) < pending_sz) > + if (curr_write_sz <= pending_sz) > return; > + > + vmbus_setevent(channel); > } > > - vmbus_setevent(channel); > } > EXPORT_SYMBOL_GPL(hv_pkt_iter_close); > -- > 1.8.3.1
RE: [PATCH 1/1] scsi: storvsc: Increase cmd_per_lun for higher speed devices
> -Original Message- > From: Michael Kelley (EOSG) > Sent: Wednesday, January 24, 2018 2:50 PM > To: KY Srinivasan <k...@microsoft.com>; Stephen Hemminger > <sthem...@microsoft.com>; martin.peter...@oracle.com; Long Li > <lon...@microsoft.com>; j...@linux.vnet.ibm.com; > de...@linuxdriverproject.org; linux-kernel@vger.kernel.org; linux- > s...@vger.kernel.org > Cc: Michael Kelley (EOSG) <michael.h.kel...@microsoft.com> > Subject: [PATCH 1/1] scsi: storvsc: Increase cmd_per_lun for higher speed > devices > > Increase cmd_per_lun to allow more I/Os in progress per device, > particularly for NVMe's. The Hyper-V host side can handle the > higher count with no issues. > > Signed-off-by: Michael Kelley <mikel...@microsoft.com> Reviewed-by: K. Y. Srinivasan <k...@microsoft.com> Acked-by: K. Y. Srinivasan <k...@microsoft.com> Thanks, K. Y > --- > drivers/scsi/storvsc_drv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c > index f3264c4..6205107 100644 > --- a/drivers/scsi/storvsc_drv.c > +++ b/drivers/scsi/storvsc_drv.c > @@ -1657,7 +1657,7 @@ static int storvsc_queuecommand(struct Scsi_Host > *host, struct scsi_cmnd *scmnd) > .eh_timed_out = storvsc_eh_timed_out, > .slave_alloc = storvsc_device_alloc, > .slave_configure = storvsc_device_configure, > - .cmd_per_lun = 255, > + .cmd_per_lun = 2048, > .this_id = -1, > .use_clustering = ENABLE_CLUSTERING, > /* Make sure we dont get a sg segment crosses a page boundary */ > -- > 1.8.3.1
RE: [PATCH 1/1] scsi: storvsc: Increase cmd_per_lun for higher speed devices
> -Original Message- > From: Michael Kelley (EOSG) > Sent: Wednesday, January 24, 2018 2:50 PM > To: KY Srinivasan ; Stephen Hemminger > ; martin.peter...@oracle.com; Long Li > ; j...@linux.vnet.ibm.com; > de...@linuxdriverproject.org; linux-kernel@vger.kernel.org; linux- > s...@vger.kernel.org > Cc: Michael Kelley (EOSG) > Subject: [PATCH 1/1] scsi: storvsc: Increase cmd_per_lun for higher speed > devices > > Increase cmd_per_lun to allow more I/Os in progress per device, > particularly for NVMe's. The Hyper-V host side can handle the > higher count with no issues. > > Signed-off-by: Michael Kelley Reviewed-by: K. Y. Srinivasan Acked-by: K. Y. Srinivasan Thanks, K. Y > --- > drivers/scsi/storvsc_drv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c > index f3264c4..6205107 100644 > --- a/drivers/scsi/storvsc_drv.c > +++ b/drivers/scsi/storvsc_drv.c > @@ -1657,7 +1657,7 @@ static int storvsc_queuecommand(struct Scsi_Host > *host, struct scsi_cmnd *scmnd) > .eh_timed_out = storvsc_eh_timed_out, > .slave_alloc = storvsc_device_alloc, > .slave_configure = storvsc_device_configure, > - .cmd_per_lun = 255, > + .cmd_per_lun = 2048, > .this_id = -1, > .use_clustering = ENABLE_CLUSTERING, > /* Make sure we dont get a sg segment crosses a page boundary */ > -- > 1.8.3.1