* Re: [PATCH v2] video: hyperv: hyperv_fb: Use physical memory for fb on HyperV Gen 1 VMs.
From: kbuild test robot @ 2019-11-25 13:07 UTC (permalink / raw)
To: Wei Hu
Cc: kbuild-all, b.zolnierkie, kys, haiyangz, sthemmin, sashal, hch,
m.szyprowski, mchehab+samsung, sam, gregkh, alexandre.belloni,
info, arnd, dri-devel, linux-fbdev, linux-kernel, linux-hyperv,
decui, mikelley, Wei Hu
In-Reply-To: <20191122082408.3210-1-weh@microsoft.com>
[-- Attachment #1: Type: text/plain, Size: 10810 bytes --]
Hi Wei,
I love your patch! Yet something to improve:
[auto build test ERROR on next-20191122]
[cannot apply to linus/master v5.4-rc8 v5.4-rc7 v5.4-rc6 v5.4]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Wei-Hu/video-hyperv-hyperv_fb-Use-physical-memory-for-fb-on-HyperV-Gen-1-VMs/20191124-163533
base: b9d3d01405061bb42358fe53f824e894a1922ced
config: i386-randconfig-b003-20191125 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-14) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
ld: kernel/dma/contiguous.o: in function `dma_alloc_from_contiguous':
>> kernel/dma/contiguous.c:199: undefined reference to `cma_alloc'
>> ld: kernel/dma/contiguous.c:199: undefined reference to `cma_alloc'
ld: kernel/dma/contiguous.o: in function `dma_release_from_contiguous':
>> kernel/dma/contiguous.c:215: undefined reference to `cma_release'
>> ld: kernel/dma/contiguous.c:215: undefined reference to `cma_release'
ld: kernel/dma/contiguous.o: in function `dma_alloc_contiguous':
kernel/dma/contiguous.c:248: undefined reference to `cma_alloc'
ld: kernel/dma/contiguous.o: in function `dma_free_contiguous':
kernel/dma/contiguous.c:267: undefined reference to `cma_release'
ld: kernel/dma/contiguous.c:267: undefined reference to `cma_release'
ld: kernel/dma/contiguous.o: in function `dma_contiguous_reserve_area':
>> kernel/dma/contiguous.c:169: undefined reference to `cma_declare_contiguous'
>> ld: kernel/dma/contiguous.c:175: undefined reference to `cma_get_size'
>> ld: kernel/dma/contiguous.c:175: undefined reference to `cma_get_base'
vim +199 kernel/dma/contiguous.c
c64be2bb1c6eb4 drivers/base/dma-contiguous.c Marek Szyprowski 2011-12-29 145
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 146 /**
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 147 * dma_contiguous_reserve_area() - reserve custom contiguous area
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 148 * @size: Size of the reserved area (in bytes),
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 149 * @base: Base address of the reserved area optional, use 0 for any
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 150 * @limit: End address of the reserved memory (optional, 0 for any).
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 151 * @res_cma: Pointer to store the created cma region.
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 152 * @fixed: hint about where to place the reserved area
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 153 *
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 154 * This function reserves memory from early allocator. It should be
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 155 * called by arch specific code once the early allocator (memblock or bootmem)
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 156 * has been activated and all other subsystems have already allocated/reserved
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 157 * memory. This function allows to create custom reserved areas for specific
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 158 * devices.
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 159 *
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 160 * If @fixed is true, reserve contiguous area at exactly @base. If false,
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 161 * reserve in range from @base to @limit.
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 162 */
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 163 int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base,
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 164 phys_addr_t limit, struct cma **res_cma,
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 165 bool fixed)
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 166 {
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 167 int ret;
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 168
f318dd083c8128 drivers/base/dma-contiguous.c Laura Abbott 2017-04-18 @169 ret = cma_declare_contiguous(base, size, limit, 0, 0, fixed,
f318dd083c8128 drivers/base/dma-contiguous.c Laura Abbott 2017-04-18 170 "reserved", res_cma);
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 171 if (ret)
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 172 return ret;
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 173
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 174 /* Architecture specific contiguous memory fixup. */
a254129e8686bf drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 @175 dma_contiguous_early_fixup(cma_get_base(*res_cma),
a254129e8686bf drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 176 cma_get_size(*res_cma));
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 177
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 178 return 0;
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 179 }
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 180
c64be2bb1c6eb4 drivers/base/dma-contiguous.c Marek Szyprowski 2011-12-29 181 /**
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 182 * dma_alloc_from_contiguous() - allocate pages from contiguous area
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 183 * @dev: Pointer to device for which the allocation is performed.
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 184 * @count: Requested number of pages.
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 185 * @align: Requested alignment of pages (in PAGE_SIZE order).
d834c5ab83febf kernel/dma/contiguous.c Marek Szyprowski 2018-08-17 186 * @no_warn: Avoid printing message about failed allocation.
c64be2bb1c6eb4 drivers/base/dma-contiguous.c Marek Szyprowski 2011-12-29 187 *
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 188 * This function allocates memory buffer for specified device. It uses
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 189 * device specific contiguous memory area if available or the default
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 190 * global one. Requires architecture specific dev_get_cma_area() helper
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 191 * function.
c64be2bb1c6eb4 drivers/base/dma-contiguous.c Marek Szyprowski 2011-12-29 192 */
67a2e213e7e937 drivers/base/dma-contiguous.c Rohit Vaswani 2015-10-22 193 struct page *dma_alloc_from_contiguous(struct device *dev, size_t count,
d834c5ab83febf kernel/dma/contiguous.c Marek Szyprowski 2018-08-17 194 unsigned int align, bool no_warn)
c64be2bb1c6eb4 drivers/base/dma-contiguous.c Marek Szyprowski 2011-12-29 195 {
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 196 if (align > CONFIG_CMA_ALIGNMENT)
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 197 align = CONFIG_CMA_ALIGNMENT;
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 198
d834c5ab83febf kernel/dma/contiguous.c Marek Szyprowski 2018-08-17 @199 return cma_alloc(dev_get_cma_area(dev), count, align, no_warn);
c64be2bb1c6eb4 drivers/base/dma-contiguous.c Marek Szyprowski 2011-12-29 200 }
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 201
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 202 /**
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 203 * dma_release_from_contiguous() - release allocated pages
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 204 * @dev: Pointer to device for which the pages were allocated.
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 205 * @pages: Allocated pages.
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 206 * @count: Number of allocated pages.
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 207 *
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 208 * This function releases memory allocated by dma_alloc_from_contiguous().
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 209 * It returns false when provided pages do not belong to contiguous area and
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 210 * true otherwise.
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 211 */
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 212 bool dma_release_from_contiguous(struct device *dev, struct page *pages,
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 213 int count)
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 214 {
a254129e8686bf drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 @215 return cma_release(dev_get_cma_area(dev), pages, count);
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 216 }
de9e14eebf33a6 drivers/base/dma-contiguous.c Marek Szyprowski 2014-10-13 217
:::::: The code at line 199 was first introduced by commit
:::::: d834c5ab83febf9624ad3b16c3c348aa1e02014c kernel/dma: remove unsupported gfp_mask parameter from dma_alloc_from_contiguous()
:::::: TO: Marek Szyprowski <m.szyprowski@samsung.com>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35695 bytes --]
^ permalink raw reply
* RE: [PATCH v3 2/4] PCI: hv: Add the support of hibernation
From: Michael Kelley @ 2019-11-25 22:49 UTC (permalink / raw)
To: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, lorenzo.pieralisi@arm.com, bhelgaas@google.com,
linux-hyperv@vger.kernel.org, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, Sasha Levin
In-Reply-To: <1574660034-98780-3-git-send-email-decui@microsoft.com>
From: Dexuan Cui <decui@microsoft.com> Sent: Sunday, November 24, 2019 9:34 PM
>
> Add suspend() and resume() functions so that Hyper-V virtual PCI devices are
> handled properly when the VM hibernates and resumes from hibernation.
>
> Note that the suspend() function must make sure there are no pending work
> items before calling vmbus_close(), since it runs in a process context as a
> callback in dpm_suspend(). When it starts to run, the channel callback
> hv_pci_onchannelcallback(), which runs in a tasklet context, can be still running
> concurrently and scheduling new work items onto hbus->wq in
> hv_pci_devices_present() and hv_pci_eject_device(), and the work item
> handlers can access the vmbus channel, which can be being closed by
> hv_pci_suspend(), e.g. the work item handler pci_devices_present_work() ->
> new_pcichild_device() writes to the vmbus channel.
>
> To eliminate the race, hv_pci_suspend() disables the channel callback
> tasklet, sets hbus->state to hv_pcibus_removing, and re-enables the tasklet.
> This way, when hv_pci_suspend() proceeds, it knows that no new work item
> can be scheduled, and then it flushes hbus->wq and safely closes the vmbus
> channel.
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* RE: [PATCH v3 4/4] PCI: hv: Avoid a kmemleak false positive caused by the hbus buffer
From: Michael Kelley @ 2019-11-25 22:52 UTC (permalink / raw)
To: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, lorenzo.pieralisi@arm.com, bhelgaas@google.com,
linux-hyperv@vger.kernel.org, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, Sasha Levin
In-Reply-To: <1574660034-98780-5-git-send-email-decui@microsoft.com>
From: Dexuan Cui <decui@microsoft.com> Sent: Sunday, November 24, 2019 9:34 PM
>
> With the recent 59bb47985c1d ("mm, sl[aou]b: guarantee natural
> alignment for kmalloc(power-of-two)"), kzalloc() is able to allocate
> a 4KB buffer that is guaranteed to be 4KB-aligned. Here the size and
> alignment of hbus is important because hbus's field
> retarget_msi_interrupt_params must not cross a 4KB page boundary.
>
> Here we prefer kzalloc to get_zeroed_page(), because a buffer
> allocated by the latter is not tracked and scanned by kmemleak, and
> hence kmemleak reports the pointer contained in the hbus buffer
> (i.e. the hpdev struct, which is created in new_pcichild_device() and
> is tracked by hbus->children) as memory leak (false positive).
>
> If the kernel doesn't have 59bb47985c1d, get_zeroed_page() *must* be
> used to allocate the hbus buffer and we can avoid the kmemleak false
> positive by using kmemleak_alloc() and kmemleak_free() to ask
> kmemleak to track and scan the hbus buffer.
>
> Reported-by: Lili Deng <v-lide@microsoft.com>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
> drivers/pci/controller/pci-hyperv.c | 24 +++++++++++++++++++++---
> 1 file changed, 21 insertions(+), 3 deletions(-)
>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* [PATCH] clocksource: hyperv: Reserve PAGE_SIZE space for tsc page
From: Boqun Feng @ 2019-11-26 2:17 UTC (permalink / raw)
To: linux-hyperv
Cc: Michael Kelley, Vincenzo Frascino, Boqun Feng, K. Y. Srinivasan,
Haiyang Zhang, Stephen Hemminger, Sasha Levin, Daniel Lezcano,
Thomas Gleixner, linux-kernel
Currently, the reserved size for a tsc page is 4K, which is enough for
communicating with hypervisor. However, in the case where we want to
export the tsc page to userspace (e.g. for vDSO to read the
clocksource), the tsc page should be at least PAGE_SIZE, otherwise, when
PAGE_SIZE is larger than 4K, extra kernel data will be mapped into
userspace, which means leaking kernel information.
Therefore reserve PAGE_SIZE space for tsc_pg as a preparation for the
vDSO support of ARM64 in the future. Also, while at it, replace all
reference to tsc_pg with hv_get_tsc_page() since it should be the only
interface to access tsc page.
Signed-off-by: Boqun Feng (Microsoft) <boqun.feng@gmail.com>
Cc: linux-hyperv@vger.kernel.org
---
drivers/clocksource/hyperv_timer.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
index 2317d4e3daaf..bcac936fa62b 100644
--- a/drivers/clocksource/hyperv_timer.c
+++ b/drivers/clocksource/hyperv_timer.c
@@ -213,17 +213,20 @@ EXPORT_SYMBOL_GPL(hv_stimer_global_cleanup);
struct clocksource *hyperv_cs;
EXPORT_SYMBOL_GPL(hyperv_cs);
-static struct ms_hyperv_tsc_page tsc_pg __aligned(PAGE_SIZE);
+static union {
+ struct ms_hyperv_tsc_page page;
+ u8 reserved[PAGE_SIZE];
+} tsc_pg __aligned(PAGE_SIZE);
struct ms_hyperv_tsc_page *hv_get_tsc_page(void)
{
- return &tsc_pg;
+ return &tsc_pg.page;
}
EXPORT_SYMBOL_GPL(hv_get_tsc_page);
static u64 notrace read_hv_clock_tsc(struct clocksource *arg)
{
- u64 current_tick = hv_read_tsc_page(&tsc_pg);
+ u64 current_tick = hv_read_tsc_page(hv_get_tsc_page());
if (current_tick == U64_MAX)
hv_get_time_ref_count(current_tick);
@@ -278,7 +281,7 @@ static bool __init hv_init_tsc_clocksource(void)
return false;
hyperv_cs = &hyperv_cs_tsc;
- phys_addr = virt_to_phys(&tsc_pg);
+ phys_addr = virt_to_phys(hv_get_tsc_page());
/*
* The Hyper-V TLFS specifies to preserve the value of reserved
--
2.24.0
^ permalink raw reply related
* Re: [PATCH v3 0/4] Enhance pci-hyperv to support hibernation, and 2 misc fixes
From: Lorenzo Pieralisi @ 2019-11-26 10:46 UTC (permalink / raw)
To: Dexuan Cui
Cc: kys, haiyangz, sthemmin, sashal, bhelgaas, linux-hyperv,
linux-pci, linux-kernel, mikelley, Alexander.Levin
In-Reply-To: <1574660034-98780-1-git-send-email-decui@microsoft.com>
On Sun, Nov 24, 2019 at 09:33:50PM -0800, Dexuan Cui wrote:
> I suggest the patchset goes through the pci.git tree.
>
> Patch #1: no functional change.
> Patch #2 enhances the pci-hyperv driver to support hibernation.
> Patch #3 is unrelated to hibernation.
> Patch #4 is unrelated to hibernation.
>
> Changes in v3:
> Patch #1: Added Michael Kelley's Signed-off-by.
> Patch #2: Used a better commit log message from Michael Kelley.
> Patch #3: Added Michael Kelley's Signed-off-by.
> Patch #4: Used kzalloc() rather than get_zeroed_page()/kmemleak_alloc()/
> kmemleak_free(), and added the necessary comments.
>
> Michael, can you please review #2 and #4 again?
>
> Dexuan Cui (4):
> PCI: hv: Reorganize the code in preparation of hibernation
> PCI: hv: Add the support of hibernation
> PCI: hv: Change pci_protocol_version to per-hbus
> PCI: hv: Avoid a kmemleak false positive caused by the hbus buffer
>
> drivers/pci/controller/pci-hyperv.c | 208 ++++++++++++++++++++++++----
> 1 file changed, 179 insertions(+), 29 deletions(-)
Applied to pci/hv, should be able to hit v5.5, thanks.
Lorenzo
^ permalink raw reply
* RE: [PATCH v2] video: hyperv: hyperv_fb: Use physical memory for fb on HyperV Gen 1 VMs.
From: Michael Kelley @ 2019-11-27 18:14 UTC (permalink / raw)
To: Wei Hu, b.zolnierkie@samsung.com, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, sashal@kernel.org, hch@lst.de,
m.szyprowski@samsung.com, mchehab+samsung@kernel.org,
sam@ravnborg.org, gregkh@linuxfoundation.org,
alexandre.belloni@bootlin.com, info@metux.net, arnd@arndb.de,
dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
Dexuan Cui
In-Reply-To: <20191122082408.3210-1-weh@microsoft.com>
From: Wei Hu <weh@microsoft.com> Sent: Friday, November 22, 2019 12:24 AM
>
> On Hyper-V, Generation 1 VMs can directly use VM's physical memory for
> their framebuffers. This can improve the efficiency of framebuffer and
> overall performence for VM. The physical memory assigned to framebuffer
> must be contiguous. We use CMA allocator to get contiguouse physicial
> memory when the framebuffer size is greater than 4MB. For size under
> 4MB, we use alloc_pages to achieve this.
>
> To enable framebuffer memory allocation from CMA, supply a kernel
> parameter to give enough space to CMA allocator at boot time. For
> example:
> cma=130m
> This gives 130MB memory to CAM allocator that can be allocated to
> framebuffer. If this fails, we fall back to the old way of using
> mmio for framebuffer.
>
> Signed-off-by: Wei Hu <weh@microsoft.com>
> ---
> v2: Incorporated review comments form hch@lst.de, Michael Kelley and
> Dexuan Cui
> - Use dma_alloc_coherent to allocate large contiguous memory
> - Use phys_addr_t for physical addresses
> - Corrected a few spelling errors and minor cleanups
> - Also tested on 32 bit Ubuntu guest
>
> drivers/video/fbdev/Kconfig | 1 +
> drivers/video/fbdev/hyperv_fb.c | 196 +++++++++++++++++++++++++-------
> 2 files changed, 158 insertions(+), 39 deletions(-)
>
[snip]
> +/*
> + * Allocate enough contiguous physical memory.
> + * Return physical address if succeeded or -1 if failed.
> + */
> +static phys_addr_t hvfb_get_phymem(struct hv_device *hdev,
> + unsigned int request_size)
> +{
> + struct page *page = NULL;
> + dma_addr_t dma_handle;
> + void *vmem;
> + unsigned int request_pages;
> + phys_addr_t paddr = 0;
> + unsigned int order = get_order(request_size);
> +
> + if (request_size == 0)
> + return -1;
> +
> + /* Try to call alloc_pages if the size is less than 2^MAX_ORDER */
> + if (order < MAX_ORDER) {
> + page = alloc_pages(GFP_KERNEL | __GFP_ZERO, order);
> + if (!page)
> + return -1;
> +
> + paddr = (page_to_pfn(page) << PAGE_SHIFT);
> + request_pages = (1 << order);
> + goto get_phymem1;
> + }
Could you use an 'else' clause here and eliminate the above
'goto' statement? I know that makes the below code be indented
one level deeper, but that doesn't seem particularly problematic
here. The reason we have 'else' clauses is to avoid 'goto's and
labels. :-)
> +
> + /* Allocate from CMA */
> + if (hdev == NULL)
> + return -1;
The above test seems unnecessary. A lot of things would have
broken before getting to this function if hdev was NULL.
> +
> + hdev->device.coherent_dma_mask = DMA_BIT_MASK(64);
> +
> + request_pages = (round_up(request_size, PAGE_SIZE) >> PAGE_SHIFT);
> +
> + vmem = dma_alloc_coherent(&hdev->device,
> + request_pages * PAGE_SIZE,
> + &dma_handle,
> + GFP_KERNEL | __GFP_NOWARN);
> +
> + if (!vmem)
> + return -1;
> +
> + paddr = virt_to_phys(vmem);
> +
> +get_phymem1:
> + pr_info("Allocated %d pages starts at physical addr 0x%llx\n",
> + request_pages, paddr);
I wonder if we want to show the physical address here. The Linux kernel
definitely does not show kernel virtual addresses due to security
concerns, and I'm wondering if the same applies to physical addresses.
What's the benefit to showing the physical address?
And in the message "starts" should be "starting".
> +
> + return paddr;
> +}
> +
> +/* Release contiguous physical memory */
> +static void hvfb_release_phymem(struct hv_device *hdev,
> + phys_addr_t paddr, unsigned int size)
> +{
> + unsigned int order = get_order(size);
> +
> + if (order < MAX_ORDER)
> + __free_pages(pfn_to_page(paddr >> PAGE_SHIFT), order);
> + else
> + dma_free_coherent(&hdev->device,
> + round_up(size, PAGE_SIZE),
> + phys_to_virt(paddr),
> + paddr);
> +}
> +
>
> /* Get framebuffer memory from Hyper-V video pci space */
> static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info)
> @@ -947,8 +1028,57 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info
> *info)
> void __iomem *fb_virt;
> int gen2vm = efi_enabled(EFI_BOOT);
> resource_size_t pot_start, pot_end;
> + phys_addr_t paddr;
> int ret;
>
> + if (!gen2vm) {
> + pdev = pci_get_device(PCI_VENDOR_ID_MICROSOFT,
> + PCI_DEVICE_ID_HYPERV_VIDEO, NULL);
> + if (!pdev) {
> + pr_err("Unable to find PCI Hyper-V video\n");
> + return -ENODEV;
> + }
> + }
> +
> + info->apertures = alloc_apertures(1);
> + if (!info->apertures)
> + goto err1;
There's a small memory leak here. The apertures are never freed in any
of the error cases in this function, or in hvfb_putmem(). This is not a bug you
introduced -- the original code had the same leak.
> +
> + if (gen2vm) {
> + info->apertures->ranges[0].base = screen_info.lfb_base;
> + info->apertures->ranges[0].size = screen_info.lfb_size;
> + } else {
> + info->apertures->ranges[0].base = pci_resource_start(pdev, 0);
> + info->apertures->ranges[0].size = pci_resource_len(pdev, 0);
> + }
> +
> + /*
> + * For Gen 1 VM, we can directly use the contiguous memory
> + * from VM. If we succeed, deferred IO happens directly
> + * on this allocated framebuffer memory, avoiding extra
> + * memory copy.
> + */
> + if (!gen2vm) {
> + paddr = hvfb_get_phymem(hdev, screen_fb_size);
> + if (paddr != (phys_addr_t) -1) {
> + par->mmio_pp = paddr;
> + par->mmio_vp = par->dio_vp = __va(paddr);
> +
> + info->fix.smem_start = paddr;
> + info->fix.smem_len = screen_fb_size;
> + info->screen_base = par->mmio_vp;
> + info->screen_size = screen_fb_size;
> +
> + par->need_docopy = false;
> + goto getmem1;
Maybe change the 'getmem1' label to 'done' or something similarly
indicative having successfully completed everything that needs to be
done?
> + }
> + pr_info("Unable to allocate enough contiguous physical memory on Gen 1
> VM. Use MMIO instead.\n");
I'd suggest changing the message to say "Using MMIO instead". This is just an
informative message indicating what the driver is doing. "Use MMIO instead"
sounds like a directive to the user to do something different, like change his
kernel configuration, and that's not the intent.
> + }
In the above code, there are three, almost consecutive, tests of the "gen2vm"
variable. It seems like the apertures could be allocated first, and then the three
tests combined into one test. Then you have one range of code for Gen 1 and
another range for Gen 2 and fewer lines 'if' statements, 'else' statements, and
curly braces.
> +
> + /*
> + * Cannot use the contiguous physical memory.
> + * Allocate mmio space for framebuffer.
> + */
> dio_fb_size =
> screen_width * screen_height * screen_depth / 8;
>
> @@ -956,13 +1086,6 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info
> *info)
> pot_start = 0;
> pot_end = -1;
> } else {
> - pdev = pci_get_device(PCI_VENDOR_ID_MICROSOFT,
> - PCI_DEVICE_ID_HYPERV_VIDEO, NULL);
> - if (!pdev) {
> - pr_err("Unable to find PCI Hyper-V video\n");
> - return -ENODEV;
> - }
> -
> if (!(pci_resource_flags(pdev, 0) & IORESOURCE_MEM) ||
> pci_resource_len(pdev, 0) < screen_fb_size) {
> pr_err("Resource not available or (0x%lx < 0x%lx)\n",
> @@ -991,20 +1114,6 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info
> *info)
> if (par->dio_vp == NULL)
> goto err3;
>
> - info->apertures = alloc_apertures(1);
> - if (!info->apertures)
> - goto err4;
> -
> - if (gen2vm) {
> - info->apertures->ranges[0].base = screen_info.lfb_base;
> - info->apertures->ranges[0].size = screen_info.lfb_size;
> - remove_conflicting_framebuffers(info->apertures,
> - KBUILD_MODNAME, false);
> - } else {
> - info->apertures->ranges[0].base = pci_resource_start(pdev, 0);
> - info->apertures->ranges[0].size = pci_resource_len(pdev, 0);
> - }
> -
> /* Physical address of FB device */
> par->mmio_pp = par->mem->start;
> /* Virtual address of FB device */
> @@ -1015,13 +1124,15 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info
> *info)
> info->screen_base = par->dio_vp;
> info->screen_size = dio_fb_size;
>
> +getmem1:
> + remove_conflicting_framebuffers(info->apertures,
> + KBUILD_MODNAME, false);
With your change, remove_conflicting_framebuffers() is called for both
Gen 1 and Gen 2 VMs. In the old code, it was called only for Gen 2 VMs.
Is this change intentional? If so, why? I haven't delved into the details
of what remove_conflicting_framebuffers() does, so my question is
more of a double-check rather than my definitely thinking something
is wrong.
> +
> if (!gen2vm)
> pci_dev_put(pdev);
>
> return 0;
>
> -err4:
> - vfree(par->dio_vp);
> err3:
> iounmap(fb_virt);
> err2:
> @@ -1035,13 +1146,19 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info
> *info)
> }
>
> /* Release the framebuffer */
> -static void hvfb_putmem(struct fb_info *info)
> +static void hvfb_putmem(struct hv_device *hdev, struct fb_info *info)
> {
> struct hvfb_par *par = info->par;
>
> - vfree(par->dio_vp);
> - iounmap(info->screen_base);
> - vmbus_free_mmio(par->mem->start, screen_fb_size);
> + if (par->need_docopy) {
> + vfree(par->dio_vp);
> + iounmap(info->screen_base);
> + vmbus_free_mmio(par->mem->start, screen_fb_size);
> + } else {
> + hvfb_release_phymem(hdev, info->fix.smem_start,
> + screen_fb_size);
> + }
> +
> par->mem = NULL;
There's a small memory leak in the above statement. The data
structure pointed to by "mem" is not freed. The same problem
occurs in hvfb_getmem() in the "err2" path. This bug existed in
the old code as well, so it was not introduced by your changes.
Michael
^ permalink raw reply
* [GIT PULL] Hyper-V commits for v5.5
From: Sasha Levin @ 2019-11-28 15:25 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-hyperv, kys, sthemmin, linux-kernel
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
The following changes since commit a99d8080aaf358d5d23581244e5da23b35e340b9:
Linux 5.4-rc6 (2019-11-03 14:07:26 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git tags/hyperv-next-signed
for you to fetch changes up to 7a1323b5dfe44a9013a2cc56ef2973034a00bf88:
Drivers: hv: vmbus: Fix crash handler reset of Hyper-V synic (2019-11-21 20:10:46 -0500)
- ----------------------------------------------------------------
- - Support for new VMBus protocols (Andrea Parri).
- - Hibernation support (Dexuan Cui).
- - Latency testing framework (Branden Bonaby).
- - Decoupling Hyper-V page size from guest page size (Himadri Pandya).
- ----------------------------------------------------------------
Andrea Parri (3):
Drivers: hv: vmbus: Introduce table of VMBus protocol versions
Drivers: hv: vmbus: Enable VMBus protocol versions 4.1, 5.1 and 5.2
Drivers: hv: vmbus: Add module parameter to cap the VMBus version
Boqun Feng (1):
drivers: iommu: hyperv: Make HYPERV_IOMMU only available on x86
Branden Bonaby (2):
drivers: hv: vmbus: Introduce latency testing
tools: hv: add vmbus testing tool
Davidlohr Bueso (1):
drivers/hv: Replace binary semaphore with mutex
Dexuan Cui (7):
scsi: storvsc: Add the support of hibernation
video: hyperv_fb: Add the support of hibernation
hv_sock: Add the support of hibernation
hv_netvsc: Add the support of hibernation
x86/hyperv: Implement hv_is_hibernation_supported()
hv_balloon: Add the support of hibernation
HID: hyperv: Add the support of hibernation
Himadri Pandya (5):
Drivers: hv: Specify receive buffer size using Hyper-V page size
Drivers: hv: util: Specify ring buffer size using Hyper-V page size
x86: hv: Add function to allocate zeroed page for Hyper-V
Drivers: hv: vmbus: Remove dependencies on guest page size
Drivers: hv: balloon: Remove dependencies on guest page size
Michael Kelley (1):
Drivers: hv: vmbus: Fix crash handler reset of Hyper-V synic
Wei Hu (2):
video: hyperv: hyperv_fb: Obtain screen resolution from Hyper-V host
video: hyperv: hyperv_fb: Support deferred IO for Hyper-V frame buffer driver
Documentation/ABI/testing/debugfs-hyperv | 23 ++
MAINTAINERS | 1 +
arch/x86/hyperv/hv_init.c | 15 ++
arch/x86/include/asm/mshyperv.h | 1 +
drivers/hid/hid-hyperv.c | 34 +++
drivers/hv/Makefile | 1 +
drivers/hv/connection.c | 87 ++++---
drivers/hv/hv_balloon.c | 112 ++++++--
drivers/hv/hv_debugfs.c | 178 +++++++++++++
drivers/hv/hv_fcopy.c | 3 +-
drivers/hv/hv_kvp.c | 3 +-
drivers/hv/hv_snapshot.c | 3 +-
drivers/hv/hv_util.c | 13 +-
drivers/hv/hyperv_vmbus.h | 31 +++
drivers/hv/ring_buffer.c | 2 +
drivers/hv/vmbus_drv.c | 27 +-
drivers/iommu/Kconfig | 2 +-
drivers/net/hyperv/hyperv_net.h | 3 +
drivers/net/hyperv/netvsc_drv.c | 57 ++++
drivers/scsi/storvsc_drv.c | 41 +++
drivers/video/fbdev/Kconfig | 1 +
drivers/video/fbdev/hyperv_fb.c | 428 ++++++++++++++++++++++++++++---
include/asm-generic/mshyperv.h | 2 +
include/linux/hyperv.h | 31 ++-
lib/Kconfig.debug | 7 +
net/vmw_vsock/hyperv_transport.c | 20 ++
tools/hv/vmbus_testing | 376 +++++++++++++++++++++++++++
27 files changed, 1386 insertions(+), 116 deletions(-)
create mode 100644 Documentation/ABI/testing/debugfs-hyperv
create mode 100644 drivers/hv/hv_debugfs.c
create mode 100755 tools/hv/vmbus_testing
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCgAdFiEE4n5dijQDou9mhzu83qZv95d3LNwFAl3f5okACgkQ3qZv95d3
LNwLEQ//biCeM3j2rks/f2pEaY1CM2dGDHpi8wRwgnN1RNvWpc/M6JFGM/+g6a3j
U0FDmCU2mTdomM09zInq7RYW1ahHMGLO8shzvuP7LZxUdElzaaGXBVmOjFbL4NRJ
9vpDjhdpBmfR1Zy6ekLszp8NH9ZHR0FGmXt99Ljtjg8nyxBbSgBohVosbOkN55NQ
x7kLa9dPB0tm7mMCLCm00PK7+U5e520D0sML1YfMpf7FJROz+locdILI6IBRdN56
9xSSJV/hVmcGxr3tG3Aa7wQc30Hp64T2BjF9wv7yJMU98bJJE0lNP18FM/AbHi9P
YGSL9jFM26a8BFxgsBou+kLZCotk4A+MAV5jLU83g9/8tfLSBk2zVlPb1Bk6V8in
6KBP4GaBCTOa2OoL6QD/8mny+weByCaB4silgzjThBQ/BMjcQu9TpLp19GXSAZk3
dJhZkK/qYgqr4u7GOgnsVtrk2cU66YIT5+FBWt4wWhVCdvOa5BeSvYEks+eb+qFl
RUM0iI7y1BhLORztoMttLS+wrBh/Thm+UtEg2/ipI0R2JuKRsYLcbw+QATW2l02W
8mDeM94Z9KcHIMUy1MO5X+VeAfSgTzT0zBoeC5IeC5VAZNu02aoVKfLNM30vD7rY
IkdiUoLBNmKNrfbw7o+py820bFLzgYDKeDckKe2cMDXGrnykpQA=
=6xUH
-----END PGP SIGNATURE-----
^ permalink raw reply
* [RFC PATCH 0/3] vsock: support network namespace
From: Stefano Garzarella @ 2019-11-28 17:15 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, linux-hyperv, virtualization, kvm,
Michael S. Tsirkin, Stefano Garzarella, David S. Miller,
Dexuan Cui, Jason Wang, Stefan Hajnoczi, Jorgen Hansen
Hi,
now that we have multi-transport upstream, I started to take a look to
support network namespace (netns) in vsock.
As we partially discussed in the multi-transport proposal [1], it could
be nice to support network namespace in vsock to reach the following
goals:
- isolate host applications from guest applications using the same ports
with CID_ANY
- assign the same CID of VMs running in different network namespaces
- partition VMs between VMMs or at finer granularity
This preliminary implementation provides the following behavior:
- packets received from the host (received by G2H transports) are
assigned to the default netns (init_net)
- packets received from the guest (received by H2G - vhost-vsock) are
assigned to the netns of the process that opens /dev/vhost-vsock
(usually the VMM, qemu in my tests, opens the /dev/vhost-vsock)
- for vmci I need some suggestions, because I don't know how to do
and test the same in the vmci driver, for now vmci uses the
init_net
- loopback packets are exchanged only in the same netns
Questions:
1. Should we make configurable the netns (now it is init_net) where
packets from the host should be delivered?
2. Should we provide an ioctl in vhost-vsock to configure the netns
to use? (instead of using the netns of the process that opens
/dev/vhost-vsock)
3. Should we provide a way to disable the netns support in vsock?
4. Jorgen: Do you think can be useful support it in vmci host
driver?
I tested the series in this way:
l0_host$ qemu-system-x86_64 -m 4G -M accel=kvm -smp 4 \
-drive file=/tmp/vsockvm0.img,if=virtio --nographic \
-device vhost-vsock-pci,guest-cid=3
l1_vm$ ip netns add ns1
l1_vm$ ip netns add ns2
# same CID on different netns
l1_vm$ ip netns exec ns1 qemu-system-x86_64 -m 1G -M accel=kvm -smp 2 \
-drive file=/tmp/vsockvm1.img,if=virtio --nographic \
-device vhost-vsock-pci,guest-cid=4
l1_vm$ ip netns exec ns2 qemu-system-x86_64 -m 1G -M accel=kvm -smp 2 \
-drive file=/tmp/vsockvm2.img,if=virtio --nographic \
-device vhost-vsock-pci,guest-cid=4
# all iperf3 listen on CID_ANY and port 5201, but in different netns
l1_vm$ ./iperf3 --vsock -s # connection from l0 or guests started
# on default netns (init_net)
l1_vm$ ip netns exec ns1 ./iperf3 --vsock -s
l1_vm$ ip netns exec ns1 ./iperf3 --vsock -s
l0_host$ ./iperf3 --vsock -c 3
l2_vm1$ ./iperf3 --vsock -c 2
l2_vm2$ ./iperf3 --vsock -c 2
This series is on top of the vsock-loopback series (not yet merged),
and it is available in the Git repository at:
git://github.com/stefano-garzarella/linux.git vsock-netns
Any comments are really appreciated!
Thanks,
Stefano
[1] https://www.spinics.net/lists/netdev/msg575792.html
Stefano Garzarella (3):
vsock: add network namespace support
vsock/virtio_transport_common: handle netns of received packets
vhost/vsock: use netns of process that opens the vhost-vsock device
drivers/vhost/vsock.c | 29 ++++++++++++++++-------
include/linux/virtio_vsock.h | 2 ++
include/net/af_vsock.h | 6 +++--
net/vmw_vsock/af_vsock.c | 31 ++++++++++++++++++-------
net/vmw_vsock/hyperv_transport.c | 5 ++--
net/vmw_vsock/virtio_transport.c | 2 ++
net/vmw_vsock/virtio_transport_common.c | 12 ++++++++--
net/vmw_vsock/vmci_transport.c | 5 ++--
8 files changed, 67 insertions(+), 25 deletions(-)
--
2.23.0
^ permalink raw reply
* [RFC PATCH 2/3] vsock/virtio_transport_common: handle netns of received packets
From: Stefano Garzarella @ 2019-11-28 17:15 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, linux-hyperv, virtualization, kvm,
Michael S. Tsirkin, Stefano Garzarella, David S. Miller,
Dexuan Cui, Jason Wang, Stefan Hajnoczi, Jorgen Hansen
In-Reply-To: <20191128171519.203979-1-sgarzare@redhat.com>
This patch allows transports that use virtio_transport_common
to specify the network namespace where a received packet is to
be delivered.
virtio_transport and vhost_transport, for now, use the default
network namespace.
vsock_loopback uses the same network namespace of the trasmitter.
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
drivers/vhost/vsock.c | 1 +
include/linux/virtio_vsock.h | 2 ++
net/vmw_vsock/virtio_transport.c | 2 ++
net/vmw_vsock/virtio_transport_common.c | 13 ++++++++++---
4 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index dde392b91bb3..31b0f3608752 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -474,6 +474,7 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work *work)
continue;
}
+ pkt->net = vsock_default_net();
len = pkt->len;
/* Deliver to monitoring devices all received packets */
diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index 71c81e0dc8f2..a025d105a456 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -43,6 +43,7 @@ struct virtio_vsock_pkt {
struct list_head list;
/* socket refcnt not held, only use for cancellation */
struct vsock_sock *vsk;
+ struct net *net;
void *buf;
u32 buf_len;
u32 len;
@@ -53,6 +54,7 @@ struct virtio_vsock_pkt {
struct virtio_vsock_pkt_info {
u32 remote_cid, remote_port;
struct vsock_sock *vsk;
+ struct net *net;
struct msghdr *msg;
u32 pkt_len;
u16 type;
diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index dfbaf6bd8b1c..fb03a1535c21 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -527,6 +527,8 @@ static void virtio_transport_rx_work(struct work_struct *work)
}
pkt->len = len - sizeof(pkt->hdr);
+ pkt->net = vsock_default_net();
+
virtio_transport_deliver_tap_pkt(pkt);
virtio_transport_recv_pkt(&virtio_transport, pkt);
}
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 10a8cbe39f61..f249dc099c38 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -60,6 +60,7 @@ virtio_transport_alloc_pkt(struct virtio_vsock_pkt_info *info,
pkt->hdr.len = cpu_to_le32(len);
pkt->reply = info->reply;
pkt->vsk = info->vsk;
+ pkt->net = info->net;
if (info->msg && len > 0) {
pkt->buf = kmalloc(len, GFP_KERNEL);
@@ -260,6 +261,7 @@ static int virtio_transport_send_credit_update(struct vsock_sock *vsk,
.op = VIRTIO_VSOCK_OP_CREDIT_UPDATE,
.type = type,
.vsk = vsk,
+ .net = sock_net(sk_vsock(vsk)),
};
return virtio_transport_send_pkt_info(vsk, &info);
@@ -609,6 +611,7 @@ int virtio_transport_connect(struct vsock_sock *vsk)
.op = VIRTIO_VSOCK_OP_REQUEST,
.type = VIRTIO_VSOCK_TYPE_STREAM,
.vsk = vsk,
+ .net = sock_net(sk_vsock(vsk)),
};
return virtio_transport_send_pkt_info(vsk, &info);
@@ -625,6 +628,7 @@ int virtio_transport_shutdown(struct vsock_sock *vsk, int mode)
(mode & SEND_SHUTDOWN ?
VIRTIO_VSOCK_SHUTDOWN_SEND : 0),
.vsk = vsk,
+ .net = sock_net(sk_vsock(vsk)),
};
return virtio_transport_send_pkt_info(vsk, &info);
@@ -652,6 +656,7 @@ virtio_transport_stream_enqueue(struct vsock_sock *vsk,
.msg = msg,
.pkt_len = len,
.vsk = vsk,
+ .net = sock_net(sk_vsock(vsk)),
};
return virtio_transport_send_pkt_info(vsk, &info);
@@ -674,6 +679,7 @@ static int virtio_transport_reset(struct vsock_sock *vsk,
.type = VIRTIO_VSOCK_TYPE_STREAM,
.reply = !!pkt,
.vsk = vsk,
+ .net = sock_net(sk_vsock(vsk)),
};
/* Send RST only if the original pkt is not a RST pkt */
@@ -694,6 +700,7 @@ static int virtio_transport_reset_no_sock(const struct virtio_transport *t,
.op = VIRTIO_VSOCK_OP_RST,
.type = le16_to_cpu(pkt->hdr.type),
.reply = true,
+ .net = pkt->net,
};
/* Send RST only if the original pkt is not a RST pkt */
@@ -978,6 +985,7 @@ virtio_transport_send_response(struct vsock_sock *vsk,
.remote_port = le32_to_cpu(pkt->hdr.src_port),
.reply = true,
.vsk = vsk,
+ .net = sock_net(sk_vsock(vsk)),
};
return virtio_transport_send_pkt_info(vsk, &info);
@@ -1075,7 +1083,6 @@ virtio_transport_recv_listen(struct sock *sk, struct virtio_vsock_pkt *pkt,
void virtio_transport_recv_pkt(struct virtio_transport *t,
struct virtio_vsock_pkt *pkt)
{
- struct net *net = vsock_default_net();
struct sockaddr_vm src, dst;
struct vsock_sock *vsk;
struct sock *sk;
@@ -1103,9 +1110,9 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
/* The socket must be in connected or bound table
* otherwise send reset back
*/
- sk = vsock_find_connected_socket(&src, &dst, net);
+ sk = vsock_find_connected_socket(&src, &dst, pkt->net);
if (!sk) {
- sk = vsock_find_bound_socket(&dst, net);
+ sk = vsock_find_bound_socket(&dst, pkt->net);
if (!sk) {
(void)virtio_transport_reset_no_sock(t, pkt);
goto free_pkt;
--
2.23.0
^ permalink raw reply related
* [RFC PATCH 3/3] vhost/vsock: use netns of process that opens the vhost-vsock device
From: Stefano Garzarella @ 2019-11-28 17:15 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, linux-hyperv, virtualization, kvm,
Michael S. Tsirkin, Stefano Garzarella, David S. Miller,
Dexuan Cui, Jason Wang, Stefan Hajnoczi, Jorgen Hansen
In-Reply-To: <20191128171519.203979-1-sgarzare@redhat.com>
This patch assigns the network namespace of the process that opened
vhost-vsock device (e.g. VMM) to the packets coming from the guest,
allowing only host sockets in the same network namespace to
communicate with the guest.
This patch also allows to have different VMs, running in different
network namespace, with the same CID.
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
drivers/vhost/vsock.c | 30 +++++++++++++++++++++---------
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 31b0f3608752..e162b3604302 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -40,6 +40,7 @@ static DEFINE_READ_MOSTLY_HASHTABLE(vhost_vsock_hash, 8);
struct vhost_vsock {
struct vhost_dev dev;
struct vhost_virtqueue vqs[2];
+ struct net *net;
/* Link to global vhost_vsock_hash, writes use vhost_vsock_mutex */
struct hlist_node hash;
@@ -61,7 +62,7 @@ static u32 vhost_transport_get_local_cid(void)
/* Callers that dereference the return value must hold vhost_vsock_mutex or the
* RCU read lock.
*/
-static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
+static struct vhost_vsock *vhost_vsock_get(u32 guest_cid, struct net *net)
{
struct vhost_vsock *vsock;
@@ -72,7 +73,7 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
if (other_cid == 0)
continue;
- if (other_cid == guest_cid)
+ if (other_cid == guest_cid && net_eq(net, vsock->net))
return vsock;
}
@@ -245,7 +246,7 @@ vhost_transport_send_pkt(struct virtio_vsock_pkt *pkt)
rcu_read_lock();
/* Find the vhost_vsock according to guest context id */
- vsock = vhost_vsock_get(le64_to_cpu(pkt->hdr.dst_cid));
+ vsock = vhost_vsock_get(le64_to_cpu(pkt->hdr.dst_cid), pkt->net);
if (!vsock) {
rcu_read_unlock();
virtio_transport_free_pkt(pkt);
@@ -277,7 +278,8 @@ vhost_transport_cancel_pkt(struct vsock_sock *vsk)
rcu_read_lock();
/* Find the vhost_vsock according to guest context id */
- vsock = vhost_vsock_get(vsk->remote_addr.svm_cid);
+ vsock = vhost_vsock_get(vsk->remote_addr.svm_cid,
+ sock_net(sk_vsock(vsk)));
if (!vsock)
goto out;
@@ -474,7 +476,7 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work *work)
continue;
}
- pkt->net = vsock_default_net();
+ pkt->net = vsock->net;
len = pkt->len;
/* Deliver to monitoring devices all received packets */
@@ -606,7 +608,14 @@ static int vhost_vsock_dev_open(struct inode *inode, struct file *file)
vqs = kmalloc_array(ARRAY_SIZE(vsock->vqs), sizeof(*vqs), GFP_KERNEL);
if (!vqs) {
ret = -ENOMEM;
- goto out;
+ goto out_vsock;
+ }
+
+ /* Derive the network namespace from the pid opening the device */
+ vsock->net = get_net_ns_by_pid(current->pid);
+ if (IS_ERR(vsock->net)) {
+ ret = PTR_ERR(vsock->net);
+ goto out_vqs;
}
vsock->guest_cid = 0; /* no CID assigned yet */
@@ -628,7 +637,9 @@ static int vhost_vsock_dev_open(struct inode *inode, struct file *file)
vhost_work_init(&vsock->send_pkt_work, vhost_transport_send_pkt_work);
return 0;
-out:
+out_vqs:
+ kfree(vqs);
+out_vsock:
vhost_vsock_free(vsock);
return ret;
}
@@ -653,7 +664,7 @@ static void vhost_vsock_reset_orphans(struct sock *sk)
*/
/* If the peer is still valid, no need to reset connection */
- if (vhost_vsock_get(vsk->remote_addr.svm_cid))
+ if (vhost_vsock_get(vsk->remote_addr.svm_cid, sock_net(sk)))
return;
/* If the close timeout is pending, let it expire. This avoids races
@@ -701,6 +712,7 @@ static int vhost_vsock_dev_release(struct inode *inode, struct file *file)
spin_unlock_bh(&vsock->send_pkt_list_lock);
vhost_dev_cleanup(&vsock->dev);
+ put_net(vsock->net);
kfree(vsock->dev.vqs);
vhost_vsock_free(vsock);
return 0;
@@ -727,7 +739,7 @@ static int vhost_vsock_set_cid(struct vhost_vsock *vsock, u64 guest_cid)
/* Refuse if CID is already in use */
mutex_lock(&vhost_vsock_mutex);
- other = vhost_vsock_get(guest_cid);
+ other = vhost_vsock_get(guest_cid, vsock->net);
if (other && other != vsock) {
mutex_unlock(&vhost_vsock_mutex);
return -EADDRINUSE;
--
2.23.0
^ permalink raw reply related
* [RFC PATCH 1/3] vsock: add network namespace support
From: Stefano Garzarella @ 2019-11-28 17:15 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, linux-hyperv, virtualization, kvm,
Michael S. Tsirkin, Stefano Garzarella, David S. Miller,
Dexuan Cui, Jason Wang, Stefan Hajnoczi, Jorgen Hansen
In-Reply-To: <20191128171519.203979-1-sgarzare@redhat.com>
This patch adds a check of the "net" assigned to a socket during
the vsock_find_bound_socket() and vsock_find_connected_socket()
to support network namespace, allowing to share the same address
(cid, port) across different network namespaces.
G2H transports will use the default network namepsace (init_net).
H2G transports can use different network namespace for different
VMs.
This patch uses default network namepsace (init_net) in all
transports.
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
include/net/af_vsock.h | 6 +++--
net/vmw_vsock/af_vsock.c | 31 ++++++++++++++++++-------
net/vmw_vsock/hyperv_transport.c | 5 ++--
net/vmw_vsock/virtio_transport_common.c | 5 ++--
net/vmw_vsock/vmci_transport.c | 5 ++--
5 files changed, 35 insertions(+), 17 deletions(-)
diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
index b1c717286993..fb7dcf73af5b 100644
--- a/include/net/af_vsock.h
+++ b/include/net/af_vsock.h
@@ -193,13 +193,15 @@ void vsock_enqueue_accept(struct sock *listener, struct sock *connected);
void vsock_insert_connected(struct vsock_sock *vsk);
void vsock_remove_bound(struct vsock_sock *vsk);
void vsock_remove_connected(struct vsock_sock *vsk);
-struct sock *vsock_find_bound_socket(struct sockaddr_vm *addr);
+struct sock *vsock_find_bound_socket(struct sockaddr_vm *addr, struct net *net);
struct sock *vsock_find_connected_socket(struct sockaddr_vm *src,
- struct sockaddr_vm *dst);
+ struct sockaddr_vm *dst,
+ struct net *net);
void vsock_remove_sock(struct vsock_sock *vsk);
void vsock_for_each_connected_socket(void (*fn)(struct sock *sk));
int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk);
bool vsock_find_cid(unsigned int cid);
+struct net *vsock_default_net(void);
/**** TAP ****/
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 9c5b2a91baad..b485b4a4e3e9 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -226,15 +226,18 @@ static void __vsock_remove_connected(struct vsock_sock *vsk)
sock_put(&vsk->sk);
}
-static struct sock *__vsock_find_bound_socket(struct sockaddr_vm *addr)
+static struct sock *__vsock_find_bound_socket(struct sockaddr_vm *addr,
+ struct net *net)
{
struct vsock_sock *vsk;
list_for_each_entry(vsk, vsock_bound_sockets(addr), bound_table) {
- if (vsock_addr_equals_addr(addr, &vsk->local_addr))
+ if (vsock_addr_equals_addr(addr, &vsk->local_addr) &&
+ net_eq(net, sock_net(sk_vsock(vsk))))
return sk_vsock(vsk);
if (addr->svm_port == vsk->local_addr.svm_port &&
+ net_eq(net, sock_net(sk_vsock(vsk))) &&
(vsk->local_addr.svm_cid == VMADDR_CID_ANY ||
addr->svm_cid == VMADDR_CID_ANY))
return sk_vsock(vsk);
@@ -244,13 +247,15 @@ static struct sock *__vsock_find_bound_socket(struct sockaddr_vm *addr)
}
static struct sock *__vsock_find_connected_socket(struct sockaddr_vm *src,
- struct sockaddr_vm *dst)
+ struct sockaddr_vm *dst,
+ struct net *net)
{
struct vsock_sock *vsk;
list_for_each_entry(vsk, vsock_connected_sockets(src, dst),
connected_table) {
if (vsock_addr_equals_addr(src, &vsk->remote_addr) &&
+ net_eq(net, sock_net(sk_vsock(vsk))) &&
dst->svm_port == vsk->local_addr.svm_port) {
return sk_vsock(vsk);
}
@@ -295,12 +300,12 @@ void vsock_remove_connected(struct vsock_sock *vsk)
}
EXPORT_SYMBOL_GPL(vsock_remove_connected);
-struct sock *vsock_find_bound_socket(struct sockaddr_vm *addr)
+struct sock *vsock_find_bound_socket(struct sockaddr_vm *addr, struct net *net)
{
struct sock *sk;
spin_lock_bh(&vsock_table_lock);
- sk = __vsock_find_bound_socket(addr);
+ sk = __vsock_find_bound_socket(addr, net);
if (sk)
sock_hold(sk);
@@ -311,12 +316,13 @@ struct sock *vsock_find_bound_socket(struct sockaddr_vm *addr)
EXPORT_SYMBOL_GPL(vsock_find_bound_socket);
struct sock *vsock_find_connected_socket(struct sockaddr_vm *src,
- struct sockaddr_vm *dst)
+ struct sockaddr_vm *dst,
+ struct net *net)
{
struct sock *sk;
spin_lock_bh(&vsock_table_lock);
- sk = __vsock_find_connected_socket(src, dst);
+ sk = __vsock_find_connected_socket(src, dst, net);
if (sk)
sock_hold(sk);
@@ -488,6 +494,12 @@ bool vsock_find_cid(unsigned int cid)
}
EXPORT_SYMBOL_GPL(vsock_find_cid);
+struct net *vsock_default_net(void)
+{
+ return &init_net;
+}
+EXPORT_SYMBOL_GPL(vsock_default_net);
+
static struct sock *vsock_dequeue_accept(struct sock *listener)
{
struct vsock_sock *vlistener;
@@ -586,6 +598,7 @@ static int __vsock_bind_stream(struct vsock_sock *vsk,
{
static u32 port;
struct sockaddr_vm new_addr;
+ struct net *net = sock_net(sk_vsock(vsk));
if (!port)
port = LAST_RESERVED_PORT + 1 +
@@ -603,7 +616,7 @@ static int __vsock_bind_stream(struct vsock_sock *vsk,
new_addr.svm_port = port++;
- if (!__vsock_find_bound_socket(&new_addr)) {
+ if (!__vsock_find_bound_socket(&new_addr, net)) {
found = true;
break;
}
@@ -620,7 +633,7 @@ static int __vsock_bind_stream(struct vsock_sock *vsk,
return -EACCES;
}
- if (__vsock_find_bound_socket(&new_addr))
+ if (__vsock_find_bound_socket(&new_addr, net))
return -EADDRINUSE;
}
diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
index 3c7d07a99fc5..fc48a861a0bc 100644
--- a/net/vmw_vsock/hyperv_transport.c
+++ b/net/vmw_vsock/hyperv_transport.c
@@ -201,7 +201,8 @@ static void hvs_remote_addr_init(struct sockaddr_vm *remote,
remote->svm_port = host_ephemeral_port++;
- sk = vsock_find_connected_socket(remote, local);
+ sk = vsock_find_connected_socket(remote, local,
+ vsock_default_net());
if (!sk) {
/* Found an available ephemeral port */
return;
@@ -350,7 +351,7 @@ static void hvs_open_connection(struct vmbus_channel *chan)
return;
hvs_addr_init(&addr, conn_from_host ? if_type : if_instance);
- sk = vsock_find_bound_socket(&addr);
+ sk = vsock_find_bound_socket(&addr, vsock_default_net());
if (!sk)
return;
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 0e20b0f6eb65..10a8cbe39f61 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -1075,6 +1075,7 @@ virtio_transport_recv_listen(struct sock *sk, struct virtio_vsock_pkt *pkt,
void virtio_transport_recv_pkt(struct virtio_transport *t,
struct virtio_vsock_pkt *pkt)
{
+ struct net *net = vsock_default_net();
struct sockaddr_vm src, dst;
struct vsock_sock *vsk;
struct sock *sk;
@@ -1102,9 +1103,9 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
/* The socket must be in connected or bound table
* otherwise send reset back
*/
- sk = vsock_find_connected_socket(&src, &dst);
+ sk = vsock_find_connected_socket(&src, &dst, net);
if (!sk) {
- sk = vsock_find_bound_socket(&dst);
+ sk = vsock_find_bound_socket(&dst, net);
if (!sk) {
(void)virtio_transport_reset_no_sock(t, pkt);
goto free_pkt;
diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
index 4b8b1150a738..3ad15d51b30b 100644
--- a/net/vmw_vsock/vmci_transport.c
+++ b/net/vmw_vsock/vmci_transport.c
@@ -669,6 +669,7 @@ static bool vmci_transport_stream_allow(u32 cid, u32 port)
static int vmci_transport_recv_stream_cb(void *data, struct vmci_datagram *dg)
{
+ struct net *net = vsock_default_net();
struct sock *sk;
struct sockaddr_vm dst;
struct sockaddr_vm src;
@@ -702,9 +703,9 @@ static int vmci_transport_recv_stream_cb(void *data, struct vmci_datagram *dg)
vsock_addr_init(&src, pkt->dg.src.context, pkt->src_port);
vsock_addr_init(&dst, pkt->dg.dst.context, pkt->dst_port);
- sk = vsock_find_connected_socket(&src, &dst);
+ sk = vsock_find_connected_socket(&src, &dst, net);
if (!sk) {
- sk = vsock_find_bound_socket(&dst);
+ sk = vsock_find_bound_socket(&dst, net);
if (!sk) {
/* We could not find a socket for this specified
* address. If this packet is a RST, we just drop it.
--
2.23.0
^ permalink raw reply related
* RE: [EXTERNAL] [PATCH 1/2] PCI: hv: decouple the func definition in hv_dr_state from VSP message
From: Michael Kelley @ 2019-11-30 4:30 UTC (permalink / raw)
To: longli@linuxonhyperv.com, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, Sasha Levin, Lorenzo Pieralisi, Andrew Murray,
Bjorn Helgaas, linux-hyperv@vger.kernel.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Long Li
In-Reply-To: <1574474229-44840-1-git-send-email-longli@linuxonhyperv.com>
From: longli@linuxonhyperv.com Sent: Friday, November 22, 2019 5:57 PM
>
> From: Long Li <longli@microsoft.com>
>
> hv_dr_state is used to find present PCI devices on the bus. The structure
> reuses struct pci_function_description from VSP message to describe a device.
>
> To prepare support for pci_function_description v2, we need to decouple this
> dependence in hv_dr_state so it can work with both v1 and v2 VSP messages.
>
> There is no functionality change.
>
> Signed-off-by: Long Li <longli@microsoft.com>
> ---
> drivers/pci/controller/pci-hyperv.c | 100 +++++++++++++++++++---------
> 1 file changed, 69 insertions(+), 31 deletions(-)
>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* RE: [EXTERNAL] [PATCH 2/2] PCI: hv: Add support for protocol 1.3 and support PCI_BUS_RELATIONS2
From: Michael Kelley @ 2019-11-30 4:45 UTC (permalink / raw)
To: longli@linuxonhyperv.com, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, Sasha Levin, Lorenzo Pieralisi, Andrew Murray,
Bjorn Helgaas, linux-hyperv@vger.kernel.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
Dexuan Cui
Cc: Long Li
In-Reply-To: <1574474229-44840-2-git-send-email-longli@linuxonhyperv.com>
From: longli@linuxonhyperv.com Sent: Friday, November 22, 2019 5:57 PM
>
> From: Long Li <longli@microsoft.com>
>
> Starting with Hyper-V PCI protocol version 1.3, the host VSP can send
> PCI_BUS_RELATIONS2 and pass the vNUMA node information for devices on the bus.
> The vNUMA node tells which guest NUMA node this device is on based on guest
> VM configuration topology and physical device inforamtion.
>
> The patch adds code to negotiate v1.3 and process PCI_BUS_RELATIONS2.
>
> Signed-off-by: Long Li <longli@microsoft.com>
> ---
> drivers/pci/controller/pci-hyperv.c | 107 ++++++++++++++++++++++++++++
> 1 file changed, 107 insertions(+)
>
[snip]
> +/*
> + * Set NUMA node for the devices on the bus
> + */
> +static void pci_assign_numa_node(struct hv_pcibus_device *hbus)
> +{
> + struct pci_dev *dev;
> + struct pci_bus *bus = hbus->pci_bus;
> + struct hv_pci_dev *hv_dev;
> +
> + list_for_each_entry(dev, &bus->devices, bus_list) {
> + hv_dev = get_pcichild_wslot(hbus, devfn_to_wslot(dev->devfn));
> + if (!hv_dev)
> + continue;
> +
> + if (hv_dev->desc.flags & HV_PCI_DEVICE_FLAG_NUMA_AFFINITY)
> + set_dev_node(&dev->dev, hv_dev->desc.virtual_numa_node);
> + }
> +}
> +
get_pcichild_wslot() gets a reference to the hv_dev, so a call to put_pcichild() is
needed to balance.
But more broadly, is the call to set_dev_node() operating on the correct
struct device? There's a struct device in the struct hv_device, and also one in the
struct pci_dev. Everything in this module seems to be operating on the former.
For example, all the dev_err() calls identify the struct device in struct hv_device.
And enumerating all the devices on a virtual PCI bus is done by iterating through
the hbus->children list, not the bus->devices list. I don't completely understand
the interplay between the two struct device entries, but the difference makes
me wonder if the above code should be setting the NUMA node on the struct
device that's in struct hv_device.
Michael
^ permalink raw reply
* Re: [GIT PULL] Hyper-V commits for v5.5
From: pr-tracker-bot @ 2019-11-30 23:05 UTC (permalink / raw)
To: Sasha Levin
Cc: Linus Torvalds, linux-kernel, linux-hyperv, kys, sthemmin,
linux-kernel
In-Reply-To: <20191128152532.DAF4D2168B@mail.kernel.org>
The pull request you sent on Thu, 28 Nov 2019 10:25:31 -0500:
> git://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git tags/hyperv-next-signed
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/0dd0c8f7db036b9aa61b70fa9fac423493cd5d17
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* Re: BUG: sleeping function called from invalid context in __alloc_pages_nodemask
From: syzbot @ 2019-12-02 4:07 UTC (permalink / raw)
To: a, akpm, alex.aring, allison, andrew, andy, ap420073, aryabinin,
ast, b.a.t.m.a.n, bridge, christophe.leroy, cleech, daniel, davem,
dja, dsa, dvyukov, edumazet, f.fainelli, fw, glider, gor, gregkh,
gustavo, gvaradar, haiyangz, hdanton, idosch, info, j.vosburgh, j,
jakub.kicinski, jhs, jiri, johan.hedberg, johannes.berg, jwi,
kasan-dev, kstewart, kvalo, kys, lariel, linmiaohe,
linux-bluetooth, linux-hams, linux-hyperv, linux-kernel, linux-mm,
linux-ppp
In-Reply-To: <000000000000c280ba05988b6242@google.com>
syzbot has bisected this bug to:
commit ab92d68fc22f9afab480153bd82a20f6e2533769
Author: Taehee Yoo <ap420073@gmail.com>
Date: Mon Oct 21 18:47:51 2019 +0000
net: core: add generic lockdep keys
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=15769712e00000
start commit: 419593da Add linux-next specific files for 20191129
git tree: linux-next
final crash: https://syzkaller.appspot.com/x/report.txt?x=17769712e00000
console output: https://syzkaller.appspot.com/x/log.txt?x=13769712e00000
kernel config: https://syzkaller.appspot.com/x/.config?x=7c04b0959e75c206
dashboard link: https://syzkaller.appspot.com/bug?extid=4925d60532bf4c399608
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=16148e9ce00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=12a1f786e00000
Reported-by: syzbot+4925d60532bf4c399608@syzkaller.appspotmail.com
Fixes: ab92d68fc22f ("net: core: add generic lockdep keys")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
^ permalink raw reply
* RE: [EXTERNAL] [PATCH 2/2] PCI: hv: Add support for protocol 1.3 and support PCI_BUS_RELATIONS2
From: Long Li @ 2019-12-02 21:23 UTC (permalink / raw)
To: Michael Kelley, longli@linuxonhyperv.com, KY Srinivasan,
Haiyang Zhang, Stephen Hemminger, Sasha Levin, Lorenzo Pieralisi,
Andrew Murray, Bjorn Helgaas, linux-hyperv@vger.kernel.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
Dexuan Cui
In-Reply-To: <CY4PR21MB0629300E161C5119D4714A64D7410@CY4PR21MB0629.namprd21.prod.outlook.com>
>Subject: RE: [EXTERNAL] [PATCH 2/2] PCI: hv: Add support for protocol 1.3 and
>support PCI_BUS_RELATIONS2
>
>From: longli@linuxonhyperv.com Sent: Friday, November 22, 2019 5:57 PM
>>
>> From: Long Li <longli@microsoft.com>
>>
>> Starting with Hyper-V PCI protocol version 1.3, the host VSP can send
>> PCI_BUS_RELATIONS2 and pass the vNUMA node information for devices
>on the bus.
>> The vNUMA node tells which guest NUMA node this device is on based on
>> guest VM configuration topology and physical device inforamtion.
>>
>> The patch adds code to negotiate v1.3 and process PCI_BUS_RELATIONS2.
>>
>> Signed-off-by: Long Li <longli@microsoft.com>
>> ---
>> drivers/pci/controller/pci-hyperv.c | 107
>> ++++++++++++++++++++++++++++
>> 1 file changed, 107 insertions(+)
>>
>
>[snip]
>
>> +/*
>> + * Set NUMA node for the devices on the bus */ static void
>> +pci_assign_numa_node(struct hv_pcibus_device *hbus) {
>> + struct pci_dev *dev;
>> + struct pci_bus *bus = hbus->pci_bus;
>> + struct hv_pci_dev *hv_dev;
>> +
>> + list_for_each_entry(dev, &bus->devices, bus_list) {
>> + hv_dev = get_pcichild_wslot(hbus, devfn_to_wslot(dev-
>>devfn));
>> + if (!hv_dev)
>> + continue;
>> +
>> + if (hv_dev->desc.flags &
>HV_PCI_DEVICE_FLAG_NUMA_AFFINITY)
>> + set_dev_node(&dev->dev, hv_dev-
>>desc.virtual_numa_node);
>> + }
>> +}
>> +
>
>get_pcichild_wslot() gets a reference to the hv_dev, so a call to put_pcichild()
>is needed to balance.
Thanks for pointing this out! I will send v2 to fix this.
>
>But more broadly, is the call to set_dev_node() operating on the correct struct
>device? There's a struct device in the struct hv_device, and also one in the
>struct pci_dev. Everything in this module seems to be operating on the
>former.
>For example, all the dev_err() calls identify the struct device in struct
>hv_device.
>And enumerating all the devices on a virtual PCI bus is done by iterating
>through the hbus->children list, not the bus->devices list. I don't completely
>understand the interplay between the two struct device entries, but the
>difference makes me wonder if the above code should be setting the NUMA
>node on the struct device that's in struct hv_device.
There are two "bus" variables in this function. "bus" is a "struct pci_bus" from the PCI layer. "hbus" is a "struct hv_pcibus_device" defined in pci-hyperv.
The parameter passed to set_dev_node is &dev->dev, the dev is enumerated from bus->devices. So dev (struct pci_dev) is from the PCI layer, this function sets the node on the device that will be used to probe and load its corresponding driver.
There is a separate list of hbus->children. It's represents pci-hyperv's view of devices on its bus. pci-hyperv presents those devices to PCI layer when pci_scan_child_bus() is called.
Long
^ permalink raw reply
* RE: [EXTERNAL] [PATCH 1/2] PCI: hv: decouple the func definition in hv_dr_state from VSP message
From: Dexuan Cui @ 2019-12-02 23:29 UTC (permalink / raw)
To: longli@linuxonhyperv.com, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, Sasha Levin, Lorenzo Pieralisi, Andrew Murray,
Bjorn Helgaas, linux-hyperv@vger.kernel.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Long Li
In-Reply-To: <1574474229-44840-1-git-send-email-longli@linuxonhyperv.com>
> From: linux-hyperv-owner@vger.kernel.org
> Sent: Friday, November 22, 2019 5:57 PM
> ...
> +struct hv_pcidev_description {
> + u16 v_id; /* vendor ID */
> + u16 d_id; /* device ID */
> + u8 rev;
> + u8 prog_intf;
> + u8 subclass;
> + u8 base_class;
> + u32 subsystem_id;
> + union win_slot_encoding win_slot;
Change the spact to a TAB? :-)
> /**
> - * hv_pci_devices_present() - Handles list of new children
> + * hv_pci_start_relations_work() - Queue work to start device discovery
> * @hbus: Root PCI bus, as understood by this driver
> - * @relations: Packet from host listing children
> + * @dr: The list of children returned from host
> *
> - * This function is invoked whenever a new list of devices for
> - * this bus appears.
> + * Return: 0 on success, 1 on failure
> */
Usually we return a negative value upon error, if possible.
> -static void hv_pci_devices_present(struct hv_pcibus_device *hbus,
> - struct pci_bus_relations *relations)
> +static int hv_pci_start_relations_work(struct hv_pcibus_device *hbus,
> + struct hv_dr_state *dr)
> {
> - struct hv_dr_state *dr;
> struct hv_dr_work *dr_wrk;
> - unsigned long flags;
> bool pending_dr;
> + unsigned long flags;
>
> dr_wrk = kzalloc(sizeof(*dr_wrk), GFP_NOWAIT);
> if (!dr_wrk)
> - return;
> -
> - dr = kzalloc(offsetof(struct hv_dr_state, func) +
> - (sizeof(struct pci_function_description) *
> - (relations->device_count)), GFP_NOWAIT);
> - if (!dr) {
> - kfree(dr_wrk);
> - return;
> - }
> + return 1;
How about "return -ENOMEM;" ?
> @@ -3018,7 +3055,7 @@ static void hv_pci_bus_exit(struct hv_device *hdev)
> struct pci_packet teardown_packet;
> u8 buffer[sizeof(struct pci_message)];
> } pkt;
> - struct pci_bus_relations relations;
> + struct hv_dr_state *dr;
> struct hv_pci_compl comp_pkt;
> int ret;
>
> @@ -3030,8 +3067,9 @@ static void hv_pci_bus_exit(struct hv_device *hdev)
> return;
>
> /* Delete any children which might still exist. */
> - memset(&relations, 0, sizeof(relations));
> - hv_pci_devices_present(hbus, &relations);
> + dr = kzalloc(sizeof(*dr), GFP_ATOMIC);
Here we are in a process context, so GFP_KERNEL is preferred.
> + if (dr && hv_pci_start_relations_work(hbus, dr))
> + kfree(dr);
Thanks,
-- Dexuan
^ permalink raw reply
* RE: [EXTERNAL] [PATCH 2/2] PCI: hv: Add support for protocol 1.3 and support PCI_BUS_RELATIONS2
From: Dexuan Cui @ 2019-12-02 23:59 UTC (permalink / raw)
To: longli@linuxonhyperv.com, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, Sasha Levin, Lorenzo Pieralisi, Andrew Murray,
Bjorn Helgaas, linux-hyperv@vger.kernel.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Long Li
In-Reply-To: <1574474229-44840-2-git-send-email-longli@linuxonhyperv.com>
> From: linux-hyperv-owner@vger.kernel.org
> Sent: Friday, November 22, 2019 5:57 PM
> ...
> @@ -63,6 +63,7 @@
> enum pci_protocol_version_t {
> PCI_PROTOCOL_VERSION_1_1 = PCI_MAKE_VERSION(1, 1), /* Win10
> */
> PCI_PROTOCOL_VERSION_1_2 = PCI_MAKE_VERSION(1, 2), /* RS1 */
> + PCI_PROTOCOL_VERSION_1_3 = PCI_MAKE_VERSION(1, 3), /* VB */
> };
What is "VB" ? Can we use a more meaningful name here? :-)
> +struct pci_function_description2 {
> + u16 v_id; /* vendor ID */
> + u16 d_id; /* device ID */
> + u8 rev;
> + u8 prog_intf;
> + u8 subclass;
> + u8 base_class;
> + u32 subsystem_id;
> + union win_slot_encoding win_slot;
space -> TAB?
> +/*
> + * Set NUMA node for the devices on the bus
> + */
> +static void pci_assign_numa_node(struct hv_pcibus_device *hbus)
IMO we'd better add a "hv_" prefix to this function's name, otherwise it looks
more like a generic PCI subsystem API.
> +{
> + struct pci_dev *dev;
> + struct pci_bus *bus = hbus->pci_bus;
> + struct hv_pci_dev *hv_dev;
> +
> + list_for_each_entry(dev, &bus->devices, bus_list) {
> + hv_dev = get_pcichild_wslot(hbus, devfn_to_wslot(dev->devfn));
> + if (!hv_dev)
> + continue;
> +
> + if (hv_dev->desc.flags & HV_PCI_DEVICE_FLAG_NUMA_AFFINITY)
> + set_dev_node(&dev->dev, hv_dev->desc.virtual_numa_node);
> + }
> +}
Can you please give a brief background introduction to dev->numa_node,
e.g. how is it used here? -- is it used when a PCI device driver (e.g. the mlx
driver) asks the kernel to allocate memory for DMA, or allocate MSI interrupts?
How big is the performance gain in the tests? I'm curious as it looks unclear
to me how dev->numa_node is used by the PCI subsystem.
Thanks,
-- Dexuan
^ permalink raw reply
* RE: [EXTERNAL] [PATCH 1/2] PCI: hv: decouple the func definition in hv_dr_state from VSP message
From: Long Li @ 2019-12-03 0:39 UTC (permalink / raw)
To: Dexuan Cui, longli@linuxonhyperv.com, KY Srinivasan,
Haiyang Zhang, Stephen Hemminger, Sasha Levin, Lorenzo Pieralisi,
Andrew Murray, Bjorn Helgaas, linux-hyperv@vger.kernel.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <PU1P153MB016971582C7371E29065D83DBF430@PU1P153MB0169.APCP153.PROD.OUTLOOK.COM>
>Subject: RE: [EXTERNAL] [PATCH 1/2] PCI: hv: decouple the func definition in
>hv_dr_state from VSP message
>
>> From: linux-hyperv-owner@vger.kernel.org
>> Sent: Friday, November 22, 2019 5:57 PM ...
>> +struct hv_pcidev_description {
>> + u16 v_id; /* vendor ID */
>> + u16 d_id; /* device ID */
>> + u8 rev;
>> + u8 prog_intf;
>> + u8 subclass;
>> + u8 base_class;
>> + u32 subsystem_id;
>> + union win_slot_encoding win_slot;
>
>Change the spact to a TAB? :-)
>
>> /**
>> - * hv_pci_devices_present() - Handles list of new children
>> + * hv_pci_start_relations_work() - Queue work to start device
>> + discovery
>> * @hbus: Root PCI bus, as understood by this driver
>> - * @relations: Packet from host listing children
>> + * @dr: The list of children returned from host
>> *
>> - * This function is invoked whenever a new list of devices for
>> - * this bus appears.
>> + * Return: 0 on success, 1 on failure
>> */
>
>Usually we return a negative value upon error, if possible.
>
>> -static void hv_pci_devices_present(struct hv_pcibus_device *hbus,
>> - struct pci_bus_relations *relations)
>> +static int hv_pci_start_relations_work(struct hv_pcibus_device *hbus,
>> + struct hv_dr_state *dr)
>> {
>> - struct hv_dr_state *dr;
>> struct hv_dr_work *dr_wrk;
>> - unsigned long flags;
>> bool pending_dr;
>> + unsigned long flags;
>>
>> dr_wrk = kzalloc(sizeof(*dr_wrk), GFP_NOWAIT);
>> if (!dr_wrk)
>> - return;
>> -
>> - dr = kzalloc(offsetof(struct hv_dr_state, func) +
>> - (sizeof(struct pci_function_description) *
>> - (relations->device_count)), GFP_NOWAIT);
>> - if (!dr) {
>> - kfree(dr_wrk);
>> - return;
>> - }
>> + return 1;
>
>How about "return -ENOMEM;" ?
>
>> @@ -3018,7 +3055,7 @@ static void hv_pci_bus_exit(struct hv_device
>*hdev)
>> struct pci_packet teardown_packet;
>> u8 buffer[sizeof(struct pci_message)];
>> } pkt;
>> - struct pci_bus_relations relations;
>> + struct hv_dr_state *dr;
>> struct hv_pci_compl comp_pkt;
>> int ret;
>>
>> @@ -3030,8 +3067,9 @@ static void hv_pci_bus_exit(struct hv_device
>*hdev)
>> return;
>>
>> /* Delete any children which might still exist. */
>> - memset(&relations, 0, sizeof(relations));
>> - hv_pci_devices_present(hbus, &relations);
>> + dr = kzalloc(sizeof(*dr), GFP_ATOMIC);
>
>Here we are in a process context, so GFP_KERNEL is preferred.
>
>> + if (dr && hv_pci_start_relations_work(hbus, dr))
>> + kfree(dr);
>
>Thanks,
>-- Dexuan
Thanks! I will send v2 to address those comments.
Long
^ permalink raw reply
* RE: [EXTERNAL] [PATCH 2/2] PCI: hv: Add support for protocol 1.3 and support PCI_BUS_RELATIONS2
From: Long Li @ 2019-12-03 0:49 UTC (permalink / raw)
To: Dexuan Cui, longli@linuxonhyperv.com, KY Srinivasan,
Haiyang Zhang, Stephen Hemminger, Sasha Levin, Lorenzo Pieralisi,
Andrew Murray, Bjorn Helgaas, linux-hyperv@vger.kernel.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <PU1P153MB0169CFE46920BAEEE11CA9B7BF430@PU1P153MB0169.APCP153.PROD.OUTLOOK.COM>
>Subject: RE: [EXTERNAL] [PATCH 2/2] PCI: hv: Add support for protocol 1.3 and
>support PCI_BUS_RELATIONS2
>
>> From: linux-hyperv-owner@vger.kernel.org
>> Sent: Friday, November 22, 2019 5:57 PM ...
>> @@ -63,6 +63,7 @@
>> enum pci_protocol_version_t {
>> PCI_PROTOCOL_VERSION_1_1 = PCI_MAKE_VERSION(1, 1), /*
>Win10
>> */
>> PCI_PROTOCOL_VERSION_1_2 = PCI_MAKE_VERSION(1, 2), /* RS1
>*/
>> + PCI_PROTOCOL_VERSION_1_3 = PCI_MAKE_VERSION(1, 3), /* VB
>*/
>> };
>
>What is "VB" ? Can we use a more meaningful name here? :-)
Vibranium.
>
>> +struct pci_function_description2 {
>> + u16 v_id; /* vendor ID */
>> + u16 d_id; /* device ID */
>> + u8 rev;
>> + u8 prog_intf;
>> + u8 subclass;
>> + u8 base_class;
>> + u32 subsystem_id;
>> + union win_slot_encoding win_slot;
>
>space -> TAB?
>
>> +/*
>> + * Set NUMA node for the devices on the bus */ static void
>> +pci_assign_numa_node(struct hv_pcibus_device *hbus)
>
>IMO we'd better add a "hv_" prefix to this function's name, otherwise it looks
>more like a generic PCI subsystem API.
I will send v2 to address comments above.
>
>> +{
>> + struct pci_dev *dev;
>> + struct pci_bus *bus = hbus->pci_bus;
>> + struct hv_pci_dev *hv_dev;
>> +
>> + list_for_each_entry(dev, &bus->devices, bus_list) {
>> + hv_dev = get_pcichild_wslot(hbus, devfn_to_wslot(dev-
>>devfn));
>> + if (!hv_dev)
>> + continue;
>> +
>> + if (hv_dev->desc.flags &
>HV_PCI_DEVICE_FLAG_NUMA_AFFINITY)
>> + set_dev_node(&dev->dev, hv_dev-
>>desc.virtual_numa_node);
>> + }
>> +}
>
>Can you please give a brief background introduction to dev->numa_node, e.g.
>how is it used here? -- is it used when a PCI device driver (e.g. the mlx
>driver) asks the kernel to allocate memory for DMA, or allocate MSI interrupts?
>How big is the performance gain in the tests? I'm curious as it looks unclear to
>me how dev->numa_node is used by the PCI subsystem.
numa_node can be used by a device driver (if it's numa aware) to setup its internal data structures and allocate its MSI.
As an example, you can look at "drivers/net/ethernet/mellanox/mlx4/main.c: mlx4_load_one()":
It stores the value at : dev->numa_node = dev_to_node(&pdev->dev);
after that, dev->numa_node is used through driver.
numa_node is also exported though /sys to user-mode, so a NUMA aware application (e.g. MPI) can figure out how to pin itself to selected CPUs for the best latency.
>
>Thanks,
>-- Dexuan
^ permalink raw reply
* [PATCH] PCI: hv: fix alloc_cast.cocci warnings
From: kbuild test robot @ 2019-12-03 7:34 UTC (permalink / raw)
To: Dexuan Cui
Cc: kbuild-all, Lorenzo Pieralisi, Michael Kelley, K. Y. Srinivasan,
Haiyang Zhang, Stephen Hemminger, Sasha Levin, Andrew Murray,
Bjorn Helgaas, linux-hyperv
In-Reply-To: <201912031531.3HntnmQg%lkp@intel.com>
From: kbuild test robot <lkp@intel.com>
drivers/pci/controller/pci-hyperv.c:2925:9-32: WARNING: casting value returned by memory allocation function to (struct hv_pcibus_device *) is useless.
Remove casting the values returned by memory allocation functions
like kmalloc, kzalloc, kmem_cache_alloc, kmem_cache_zalloc etc.
Semantic patch information:
This makes an effort to find cases of casting of values returned by
kmalloc, kzalloc, kcalloc, kmem_cache_alloc, kmem_cache_zalloc,
kmem_cache_alloc_node, kmalloc_node and kzalloc_node and removes
the casting as it is not required. The result in the patch case may
need some reformatting.
Generated by: scripts/coccinelle/api/alloc/alloc_cast.cocci
Fixes: 877b911a5ba0 ("PCI: hv: Avoid a kmemleak false positive caused by the hbus buffer")
CC: Dexuan Cui <decui@microsoft.com>
Signed-off-by: kbuild test robot <lkp@intel.com>
---
tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
head: 1ab75b2e415a29dba9aec94f203c6f88dbfc0ba0
commit: 877b911a5ba0733f62239055ee869f2e117b57da [13591/14582] PCI: hv: Avoid a kmemleak false positive caused by the hbus buffer
pci-hyperv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -2922,7 +2922,7 @@ static int hv_pci_probe(struct hv_device
* positive by using kmemleak_alloc() and kmemleak_free() to ask
* kmemleak to track and scan the hbus buffer.
*/
- hbus = (struct hv_pcibus_device *)kzalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
+ hbus = kzalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
if (!hbus)
return -ENOMEM;
hbus->state = hv_pcibus_init;
^ permalink raw reply
* Re: [RFC PATCH 0/3] vsock: support network namespace
From: Stefan Hajnoczi @ 2019-12-03 9:26 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, linux-hyperv, kvm, Michael S. Tsirkin, Dexuan Cui,
linux-kernel, virtualization, Stefan Hajnoczi, David S. Miller,
Jorgen Hansen
In-Reply-To: <20191128171519.203979-1-sgarzare@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 2098 bytes --]
On Thu, Nov 28, 2019 at 06:15:16PM +0100, Stefano Garzarella wrote:
> Hi,
> now that we have multi-transport upstream, I started to take a look to
> support network namespace (netns) in vsock.
>
> As we partially discussed in the multi-transport proposal [1], it could
> be nice to support network namespace in vsock to reach the following
> goals:
> - isolate host applications from guest applications using the same ports
> with CID_ANY
> - assign the same CID of VMs running in different network namespaces
> - partition VMs between VMMs or at finer granularity
>
> This preliminary implementation provides the following behavior:
> - packets received from the host (received by G2H transports) are
> assigned to the default netns (init_net)
> - packets received from the guest (received by H2G - vhost-vsock) are
> assigned to the netns of the process that opens /dev/vhost-vsock
> (usually the VMM, qemu in my tests, opens the /dev/vhost-vsock)
> - for vmci I need some suggestions, because I don't know how to do
> and test the same in the vmci driver, for now vmci uses the
> init_net
> - loopback packets are exchanged only in the same netns
>
> Questions:
> 1. Should we make configurable the netns (now it is init_net) where
> packets from the host should be delivered?
Yes, it should be possible to have multiple G2H (e.g. virtio-vsock)
devices and to assign them to different net namespaces. Something like
net/core/dev.c:dev_change_net_namespace() will eventually be needed.
> 2. Should we provide an ioctl in vhost-vsock to configure the netns
> to use? (instead of using the netns of the process that opens
> /dev/vhost-vsock)
Creating the vhost-vsock instance in the process' net namespace makes
sense. Maybe wait for a use case before adding an ioctl.
> 3. Should we provide a way to disable the netns support in vsock?
The code should follow CONFIG_NET_NS semantics. I'm not sure what they
are exactly since struct net is always defined, regardless of whether
network namespaces are enabled.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 0/3] vsock: support network namespace
From: Stefano Garzarella @ 2019-12-03 11:17 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: netdev, linux-hyperv, kvm, Michael S. Tsirkin, Dexuan Cui,
linux-kernel, virtualization, Stefan Hajnoczi, David S. Miller,
Jorgen Hansen
In-Reply-To: <20191203092649.GB153510@stefanha-x1.localdomain>
On Tue, Dec 03, 2019 at 09:26:49AM +0000, Stefan Hajnoczi wrote:
> On Thu, Nov 28, 2019 at 06:15:16PM +0100, Stefano Garzarella wrote:
> > Hi,
> > now that we have multi-transport upstream, I started to take a look to
> > support network namespace (netns) in vsock.
> >
> > As we partially discussed in the multi-transport proposal [1], it could
> > be nice to support network namespace in vsock to reach the following
> > goals:
> > - isolate host applications from guest applications using the same ports
> > with CID_ANY
> > - assign the same CID of VMs running in different network namespaces
> > - partition VMs between VMMs or at finer granularity
> >
> > This preliminary implementation provides the following behavior:
> > - packets received from the host (received by G2H transports) are
> > assigned to the default netns (init_net)
> > - packets received from the guest (received by H2G - vhost-vsock) are
> > assigned to the netns of the process that opens /dev/vhost-vsock
> > (usually the VMM, qemu in my tests, opens the /dev/vhost-vsock)
> > - for vmci I need some suggestions, because I don't know how to do
> > and test the same in the vmci driver, for now vmci uses the
> > init_net
> > - loopback packets are exchanged only in the same netns
> >
> > Questions:
> > 1. Should we make configurable the netns (now it is init_net) where
> > packets from the host should be delivered?
>
> Yes, it should be possible to have multiple G2H (e.g. virtio-vsock)
> devices and to assign them to different net namespaces. Something like
> net/core/dev.c:dev_change_net_namespace() will eventually be needed.
>
Make sense, but for now we support only one G2H.
How we can provide this feature to the userspace?
Should we interface vsock with ip-link(8)?
I don't know if initially we can provide through sysfs a way to set the
netns of the only G2H loaded.
> > 2. Should we provide an ioctl in vhost-vsock to configure the netns
> > to use? (instead of using the netns of the process that opens
> > /dev/vhost-vsock)
>
> Creating the vhost-vsock instance in the process' net namespace makes
> sense. Maybe wait for a use case before adding an ioctl.
>
Agree.
> > 3. Should we provide a way to disable the netns support in vsock?
>
> The code should follow CONFIG_NET_NS semantics. I'm not sure what they
> are exactly since struct net is always defined, regardless of whether
> network namespaces are enabled.
I think that if CONFIG_NET_NS is not defined, all sockets and processes
are assigned to init_net and this RFC should work in this case, but I'll
try this case before v1.
I was thinking about the Kata's use case, I don't know if they launch the
VM in a netns and even the runtime in the host runs inside the same netns.
I'll send an e-mail to kata mailing list.
Thanks,
Stefano
^ permalink raw reply
* [Patch v2 1/2] PCI: hv: decouple the func definition in hv_dr_state from VSP message
From: longli @ 2019-12-04 2:53 UTC (permalink / raw)
To: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Lorenzo Pieralisi, Andrew Murray, Bjorn Helgaas, linux-hyperv,
linux-pci, linux-kernel
Cc: Long Li
From: Long Li <longli@microsoft.com>
hv_dr_state is used to find present PCI devices on the bus. The structure
reuses struct pci_function_description from VSP message to describe a device.
To prepare support for pci_function_description v2, we need to decouple this
dependence in hv_dr_state so it can work with both v1 and v2 VSP messages.
There is no functionality change.
Signed-off-by: Long Li <longli@microsoft.com>
---
Changes
v2: changed some spaces to tabs, changed failure code to -ENOMEM
drivers/pci/controller/pci-hyperv.c | 100 +++++++++++++++++++---------
1 file changed, 69 insertions(+), 31 deletions(-)
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index f1f300218fab..8c1533be6ad0 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -507,10 +507,24 @@ struct hv_dr_work {
struct hv_pcibus_device *bus;
};
+struct hv_pcidev_description {
+ u16 v_id; /* vendor ID */
+ u16 d_id; /* device ID */
+ u8 rev;
+ u8 prog_intf;
+ u8 subclass;
+ u8 base_class;
+ u32 subsystem_id;
+ union win_slot_encoding win_slot;
+ u32 ser; /* serial number */
+ u32 flags;
+ u16 virtual_numa_node;
+};
+
struct hv_dr_state {
struct list_head list_entry;
u32 device_count;
- struct pci_function_description func[0];
+ struct hv_pcidev_description func[0];
};
enum hv_pcichild_state {
@@ -527,7 +541,7 @@ struct hv_pci_dev {
refcount_t refs;
enum hv_pcichild_state state;
struct pci_slot *pci_slot;
- struct pci_function_description desc;
+ struct hv_pcidev_description desc;
bool reported_missing;
struct hv_pcibus_device *hbus;
struct work_struct wrk;
@@ -1862,7 +1876,7 @@ static void q_resource_requirements(void *context, struct pci_response *resp,
* Return: Pointer to the new tracking struct
*/
static struct hv_pci_dev *new_pcichild_device(struct hv_pcibus_device *hbus,
- struct pci_function_description *desc)
+ struct hv_pcidev_description *desc)
{
struct hv_pci_dev *hpdev;
struct pci_child_message *res_req;
@@ -1973,7 +1987,7 @@ static void pci_devices_present_work(struct work_struct *work)
{
u32 child_no;
bool found;
- struct pci_function_description *new_desc;
+ struct hv_pcidev_description *new_desc;
struct hv_pci_dev *hpdev;
struct hv_pcibus_device *hbus;
struct list_head removed;
@@ -2090,43 +2104,26 @@ static void pci_devices_present_work(struct work_struct *work)
put_hvpcibus(hbus);
kfree(dr);
}
-
/**
- * hv_pci_devices_present() - Handles list of new children
+ * hv_pci_start_relations_work() - Queue work to start device discovery
* @hbus: Root PCI bus, as understood by this driver
- * @relations: Packet from host listing children
+ * @dr: The list of children returned from host
*
- * This function is invoked whenever a new list of devices for
- * this bus appears.
+ * Return: 0 on success, -errno on failure
*/
-static void hv_pci_devices_present(struct hv_pcibus_device *hbus,
- struct pci_bus_relations *relations)
+static int hv_pci_start_relations_work(struct hv_pcibus_device *hbus,
+ struct hv_dr_state *dr)
{
- struct hv_dr_state *dr;
struct hv_dr_work *dr_wrk;
- unsigned long flags;
bool pending_dr;
+ unsigned long flags;
dr_wrk = kzalloc(sizeof(*dr_wrk), GFP_NOWAIT);
if (!dr_wrk)
- return;
-
- dr = kzalloc(offsetof(struct hv_dr_state, func) +
- (sizeof(struct pci_function_description) *
- (relations->device_count)), GFP_NOWAIT);
- if (!dr) {
- kfree(dr_wrk);
- return;
- }
+ return -ENOMEM;
INIT_WORK(&dr_wrk->wrk, pci_devices_present_work);
dr_wrk->bus = hbus;
- dr->device_count = relations->device_count;
- if (dr->device_count != 0) {
- memcpy(dr->func, relations->func,
- sizeof(struct pci_function_description) *
- dr->device_count);
- }
spin_lock_irqsave(&hbus->device_list_lock, flags);
/*
@@ -2144,6 +2141,46 @@ static void hv_pci_devices_present(struct hv_pcibus_device *hbus,
get_hvpcibus(hbus);
queue_work(hbus->wq, &dr_wrk->wrk);
}
+
+ return 0;
+}
+
+/**
+ * hv_pci_devices_present() - Handles list of new children
+ * @hbus: Root PCI bus, as understood by this driver
+ * @relations: Packet from host listing children
+ *
+ * This function is invoked whenever a new list of devices for
+ * this bus appears.
+ */
+static void hv_pci_devices_present(struct hv_pcibus_device *hbus,
+ struct pci_bus_relations *relations)
+{
+ struct hv_dr_state *dr;
+ int i;
+
+ dr = kzalloc(offsetof(struct hv_dr_state, func) +
+ (sizeof(struct hv_pcidev_description) *
+ (relations->device_count)), GFP_NOWAIT);
+
+ if (!dr)
+ return;
+
+ dr->device_count = relations->device_count;
+ for (i = 0; i < dr->device_count; i++) {
+ dr->func[i].v_id = relations->func[i].v_id;
+ dr->func[i].d_id = relations->func[i].d_id;
+ dr->func[i].rev = relations->func[i].rev;
+ dr->func[i].prog_intf = relations->func[i].prog_intf;
+ dr->func[i].subclass = relations->func[i].subclass;
+ dr->func[i].base_class = relations->func[i].base_class;
+ dr->func[i].subsystem_id = relations->func[i].subsystem_id;
+ dr->func[i].win_slot = relations->func[i].win_slot;
+ dr->func[i].ser = relations->func[i].ser;
+ }
+
+ if (hv_pci_start_relations_work(hbus, dr))
+ kfree(dr);
}
/**
@@ -3018,7 +3055,7 @@ static void hv_pci_bus_exit(struct hv_device *hdev)
struct pci_packet teardown_packet;
u8 buffer[sizeof(struct pci_message)];
} pkt;
- struct pci_bus_relations relations;
+ struct hv_dr_state *dr;
struct hv_pci_compl comp_pkt;
int ret;
@@ -3030,8 +3067,9 @@ static void hv_pci_bus_exit(struct hv_device *hdev)
return;
/* Delete any children which might still exist. */
- memset(&relations, 0, sizeof(relations));
- hv_pci_devices_present(hbus, &relations);
+ dr = kzalloc(sizeof(*dr), GFP_KERNEL);
+ if (dr && hv_pci_start_relations_work(hbus, dr))
+ kfree(dr);
ret = hv_send_resources_released(hdev);
if (ret)
--
2.17.1
^ permalink raw reply related
* [Patch v2 2/2] PCI: hv: Add support for protocol 1.3 and support PCI_BUS_RELATIONS2
From: longli @ 2019-12-04 2:53 UTC (permalink / raw)
To: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Lorenzo Pieralisi, Andrew Murray, Bjorn Helgaas, linux-hyperv,
linux-pci, linux-kernel
Cc: Long Li
In-Reply-To: <1575428017-87914-1-git-send-email-longli@linuxonhyperv.com>
From: Long Li <longli@microsoft.com>
Starting with Hyper-V PCI protocol version 1.3, the host VSP can send
PCI_BUS_RELATIONS2 and pass the vNUMA node information for devices on the bus.
The vNUMA node tells which guest NUMA node this device is on based on guest
VM configuration topology and physical device inforamtion.
The patch adds code to negotiate v1.3 and process PCI_BUS_RELATIONS2.
Signed-off-by: Long Li <longli@microsoft.com>
---
Changes
v2: Changed some spaces to tabs, added put_pcichild() after get_pcichild_wslot(), renamed pci_assign_numa_node() to hv_pci_assign_numa_node()
drivers/pci/controller/pci-hyperv.c | 109 ++++++++++++++++++++++++++++
1 file changed, 109 insertions(+)
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 8c1533be6ad0..fee0d7fdc672 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -63,6 +63,7 @@
enum pci_protocol_version_t {
PCI_PROTOCOL_VERSION_1_1 = PCI_MAKE_VERSION(1, 1), /* Win10 */
PCI_PROTOCOL_VERSION_1_2 = PCI_MAKE_VERSION(1, 2), /* RS1 */
+ PCI_PROTOCOL_VERSION_1_3 = PCI_MAKE_VERSION(1, 3), /* Vibranium */
};
#define CPU_AFFINITY_ALL -1ULL
@@ -72,6 +73,7 @@ enum pci_protocol_version_t {
* first.
*/
static enum pci_protocol_version_t pci_protocol_versions[] = {
+ PCI_PROTOCOL_VERSION_1_3,
PCI_PROTOCOL_VERSION_1_2,
PCI_PROTOCOL_VERSION_1_1,
};
@@ -124,6 +126,7 @@ enum pci_message_type {
PCI_RESOURCES_ASSIGNED2 = PCI_MESSAGE_BASE + 0x16,
PCI_CREATE_INTERRUPT_MESSAGE2 = PCI_MESSAGE_BASE + 0x17,
PCI_DELETE_INTERRUPT_MESSAGE2 = PCI_MESSAGE_BASE + 0x18, /* unused */
+ PCI_BUS_RELATIONS2 = PCI_MESSAGE_BASE + 0x19,
PCI_MESSAGE_MAXIMUM
};
@@ -169,6 +172,26 @@ struct pci_function_description {
u32 ser; /* serial number */
} __packed;
+enum pci_device_description_flags {
+ HV_PCI_DEVICE_FLAG_NONE = 0x0,
+ HV_PCI_DEVICE_FLAG_NUMA_AFFINITY = 0x1,
+};
+
+struct pci_function_description2 {
+ u16 v_id; /* vendor ID */
+ u16 d_id; /* device ID */
+ u8 rev;
+ u8 prog_intf;
+ u8 subclass;
+ u8 base_class;
+ u32 subsystem_id;
+ union win_slot_encoding win_slot;
+ u32 ser; /* serial number */
+ u32 flags;
+ u16 virtual_numa_node;
+ u16 reserved;
+} __packed;
+
/**
* struct hv_msi_desc
* @vector: IDT entry
@@ -304,6 +327,12 @@ struct pci_bus_relations {
struct pci_function_description func[0];
} __packed;
+struct pci_bus_relations2 {
+ struct pci_incoming_message incoming;
+ u32 device_count;
+ struct pci_function_description2 func[0];
+} __packed;
+
struct pci_q_res_req_response {
struct vmpacket_descriptor hdr;
s32 status; /* negative values are failures */
@@ -1417,6 +1446,7 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
break;
case PCI_PROTOCOL_VERSION_1_2:
+ case PCI_PROTOCOL_VERSION_1_3:
size = hv_compose_msi_req_v2(&ctxt.int_pkts.v2,
dest,
hpdev->desc.win_slot.slot,
@@ -1798,6 +1828,27 @@ static void hv_pci_remove_slots(struct hv_pcibus_device *hbus)
}
}
+/*
+ * Set NUMA node for the devices on the bus
+ */
+static void hv_pci_assign_numa_node(struct hv_pcibus_device *hbus)
+{
+ struct pci_dev *dev;
+ struct pci_bus *bus = hbus->pci_bus;
+ struct hv_pci_dev *hv_dev;
+
+ list_for_each_entry(dev, &bus->devices, bus_list) {
+ hv_dev = get_pcichild_wslot(hbus, devfn_to_wslot(dev->devfn));
+ if (!hv_dev)
+ continue;
+
+ if (hv_dev->desc.flags & HV_PCI_DEVICE_FLAG_NUMA_AFFINITY)
+ set_dev_node(&dev->dev, hv_dev->desc.virtual_numa_node);
+
+ put_pcichild(hv_dev);
+ }
+}
+
/**
* create_root_hv_pci_bus() - Expose a new root PCI bus
* @hbus: Root PCI bus, as understood by this driver
@@ -1820,6 +1871,7 @@ static int create_root_hv_pci_bus(struct hv_pcibus_device *hbus)
pci_lock_rescan_remove();
pci_scan_child_bus(hbus->pci_bus);
+ hv_pci_assign_numa_node(hbus);
pci_bus_assign_resources(hbus->pci_bus);
hv_pci_assign_slots(hbus);
pci_bus_add_devices(hbus->pci_bus);
@@ -2088,6 +2140,7 @@ static void pci_devices_present_work(struct work_struct *work)
*/
pci_lock_rescan_remove();
pci_scan_child_bus(hbus->pci_bus);
+ hv_pci_assign_numa_node(hbus);
hv_pci_assign_slots(hbus);
pci_unlock_rescan_remove();
break;
@@ -2183,6 +2236,46 @@ static void hv_pci_devices_present(struct hv_pcibus_device *hbus,
kfree(dr);
}
+/**
+ * hv_pci_devices_present2() - Handles list of new children
+ * @hbus: Root PCI bus, as understood by this driver
+ * @relations2: Packet from host listing children
+ *
+ * This function is the v2 version of hv_pci_devices_present()
+ */
+static void hv_pci_devices_present2(struct hv_pcibus_device *hbus,
+ struct pci_bus_relations2 *relations)
+{
+ struct hv_dr_state *dr;
+ int i;
+
+ dr = kzalloc(offsetof(struct hv_dr_state, func) +
+ (sizeof(struct hv_pcidev_description) *
+ (relations->device_count)), GFP_NOWAIT);
+
+ if (!dr)
+ return;
+
+ dr->device_count = relations->device_count;
+ for (i = 0; i < dr->device_count; i++) {
+ dr->func[i].v_id = relations->func[i].v_id;
+ dr->func[i].d_id = relations->func[i].d_id;
+ dr->func[i].rev = relations->func[i].rev;
+ dr->func[i].prog_intf = relations->func[i].prog_intf;
+ dr->func[i].subclass = relations->func[i].subclass;
+ dr->func[i].base_class = relations->func[i].base_class;
+ dr->func[i].subsystem_id = relations->func[i].subsystem_id;
+ dr->func[i].win_slot = relations->func[i].win_slot;
+ dr->func[i].ser = relations->func[i].ser;
+ dr->func[i].flags = relations->func[i].flags;
+ dr->func[i].virtual_numa_node =
+ relations->func[i].virtual_numa_node;
+ }
+
+ if (hv_pci_start_relations_work(hbus, dr))
+ kfree(dr);
+}
+
/**
* hv_eject_device_work() - Asynchronously handles ejection
* @work: Work struct embedded in internal device struct
@@ -2288,6 +2381,7 @@ static void hv_pci_onchannelcallback(void *context)
struct pci_response *response;
struct pci_incoming_message *new_message;
struct pci_bus_relations *bus_rel;
+ struct pci_bus_relations2 *bus_rel2;
struct pci_dev_inval_block *inval;
struct pci_dev_incoming *dev_message;
struct hv_pci_dev *hpdev;
@@ -2355,6 +2449,21 @@ static void hv_pci_onchannelcallback(void *context)
hv_pci_devices_present(hbus, bus_rel);
break;
+ case PCI_BUS_RELATIONS2:
+
+ bus_rel2 = (struct pci_bus_relations2 *)buffer;
+ if (bytes_recvd <
+ offsetof(struct pci_bus_relations2, func) +
+ (sizeof(struct pci_function_description2) *
+ (bus_rel2->device_count))) {
+ dev_err(&hbus->hdev->device,
+ "bus relations v2 too small\n");
+ break;
+ }
+
+ hv_pci_devices_present2(hbus, bus_rel2);
+ break;
+
case PCI_EJECT:
dev_message = (struct pci_dev_incoming *)buffer;
--
2.17.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox