If subblock size is large (e.g. 1G) 32 bit math involving it
can overflow. Rather than try to catch all instances of that,
let's tweak block size to 64 bit.
It ripples through UAPI which is an ABI change, but it's not too late to
make it, and it will allow supporting >4Gbyte blocks while might
become necessary down the road.
Fixes: 5f1f79bbc9e26 ("virtio-mem: Paravirtualized memory hotplug")
Signed-off-by: Michael S. Tsirkin <[email protected]>
---
drivers/virtio/virtio_mem.c | 14 +++++++-------
include/uapi/linux/virtio_mem.h | 4 ++--
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
index 2f357142ea5e..7b1bece8a331 100644
--- a/drivers/virtio/virtio_mem.c
+++ b/drivers/virtio/virtio_mem.c
@@ -77,7 +77,7 @@ struct virtio_mem {
uint64_t requested_size;
/* The device block size (for communicating with the device). */
- uint32_t device_block_size;
+ uint64_t device_block_size;
/* The translated node id. NUMA_NO_NODE in case not specified. */
int nid;
/* Physical start address of the memory region. */
@@ -86,7 +86,7 @@ struct virtio_mem {
uint64_t region_size;
/* The subblock size. */
- uint32_t subblock_size;
+ uint64_t subblock_size;
/* The number of subblocks per memory block. */
uint32_t nb_sb_per_mb;
@@ -1698,9 +1698,9 @@ static int virtio_mem_init(struct virtio_mem *vm)
* - At least the device block size.
* In the worst case, a single subblock per memory block.
*/
- vm->subblock_size = PAGE_SIZE * 1u << max_t(uint32_t, MAX_ORDER - 1,
- pageblock_order);
- vm->subblock_size = max_t(uint32_t, vm->device_block_size,
+ vm->subblock_size = PAGE_SIZE * 1ul << max_t(uint32_t, MAX_ORDER - 1,
+ pageblock_order);
+ vm->subblock_size = max_t(uint64_t, vm->device_block_size,
vm->subblock_size);
vm->nb_sb_per_mb = memory_block_size_bytes() / vm->subblock_size;
@@ -1713,8 +1713,8 @@ static int virtio_mem_init(struct virtio_mem *vm)
dev_info(&vm->vdev->dev, "start address: 0x%llx", vm->addr);
dev_info(&vm->vdev->dev, "region size: 0x%llx", vm->region_size);
- dev_info(&vm->vdev->dev, "device block size: 0x%x",
- vm->device_block_size);
+ dev_info(&vm->vdev->dev, "device block size: 0x%llx",
+ (unsigned long long)vm->device_block_size);
dev_info(&vm->vdev->dev, "memory block size: 0x%lx",
memory_block_size_bytes());
dev_info(&vm->vdev->dev, "subblock size: 0x%x",
diff --git a/include/uapi/linux/virtio_mem.h b/include/uapi/linux/virtio_mem.h
index a455c488a995..a9ffe041843c 100644
--- a/include/uapi/linux/virtio_mem.h
+++ b/include/uapi/linux/virtio_mem.h
@@ -185,10 +185,10 @@ struct virtio_mem_resp {
struct virtio_mem_config {
/* Block size and alignment. Cannot change. */
- __u32 block_size;
+ __u64 block_size;
/* Valid with VIRTIO_MEM_F_ACPI_PXM. Cannot change. */
__u16 node_id;
- __u16 padding;
+ __u8 padding[6];
/* Start address of the memory region. Cannot change. */
__u64 addr;
/* Region size (maximum). Cannot change. */
--
MST
On 08.06.20 08:14, Michael S. Tsirkin wrote:
> If subblock size is large (e.g. 1G) 32 bit math involving it
> can overflow. Rather than try to catch all instances of that,
> let's tweak block size to 64 bit.
I fail to see where we could actually trigger an overflow. The reported
warning looked like a false positive to me.
>
> It ripples through UAPI which is an ABI change, but it's not too late to
> make it, and it will allow supporting >4Gbyte blocks while might
> become necessary down the road.
>
This might break cloud-hypervisor, who's already implementing this
protocol upstream (ccing Hui).
https://github.com/cloud-hypervisor/cloud-hypervisor/blob/master/vm-virtio/src/mem.rs
(blocks in the gigabyte range were never the original intention of
virtio-mem, but I am not completely opposed to that)
> Fixes: 5f1f79bbc9e26 ("virtio-mem: Paravirtualized memory hotplug")
> Signed-off-by: Michael S. Tsirkin <[email protected]>
> ---
> drivers/virtio/virtio_mem.c | 14 +++++++-------
> include/uapi/linux/virtio_mem.h | 4 ++--
> 2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
> index 2f357142ea5e..7b1bece8a331 100644
> --- a/drivers/virtio/virtio_mem.c
> +++ b/drivers/virtio/virtio_mem.c
> @@ -77,7 +77,7 @@ struct virtio_mem {
> uint64_t requested_size;
>
> /* The device block size (for communicating with the device). */
> - uint32_t device_block_size;
> + uint64_t device_block_size;
> /* The translated node id. NUMA_NO_NODE in case not specified. */
> int nid;
> /* Physical start address of the memory region. */
> @@ -86,7 +86,7 @@ struct virtio_mem {
> uint64_t region_size;
>
> /* The subblock size. */
> - uint32_t subblock_size;
> + uint64_t subblock_size;
> /* The number of subblocks per memory block. */
> uint32_t nb_sb_per_mb;
>
> @@ -1698,9 +1698,9 @@ static int virtio_mem_init(struct virtio_mem *vm)
> * - At least the device block size.
> * In the worst case, a single subblock per memory block.
> */
> - vm->subblock_size = PAGE_SIZE * 1u << max_t(uint32_t, MAX_ORDER - 1,
> - pageblock_order);
> - vm->subblock_size = max_t(uint32_t, vm->device_block_size,
> + vm->subblock_size = PAGE_SIZE * 1ul << max_t(uint32_t, MAX_ORDER - 1,
> + pageblock_order);
> + vm->subblock_size = max_t(uint64_t, vm->device_block_size,
> vm->subblock_size);
> vm->nb_sb_per_mb = memory_block_size_bytes() / vm->subblock_size;
>
> @@ -1713,8 +1713,8 @@ static int virtio_mem_init(struct virtio_mem *vm)
>
> dev_info(&vm->vdev->dev, "start address: 0x%llx", vm->addr);
> dev_info(&vm->vdev->dev, "region size: 0x%llx", vm->region_size);
> - dev_info(&vm->vdev->dev, "device block size: 0x%x",
> - vm->device_block_size);
> + dev_info(&vm->vdev->dev, "device block size: 0x%llx",
> + (unsigned long long)vm->device_block_size);
> dev_info(&vm->vdev->dev, "memory block size: 0x%lx",
> memory_block_size_bytes());
> dev_info(&vm->vdev->dev, "subblock size: 0x%x",
> diff --git a/include/uapi/linux/virtio_mem.h b/include/uapi/linux/virtio_mem.h
> index a455c488a995..a9ffe041843c 100644
> --- a/include/uapi/linux/virtio_mem.h
> +++ b/include/uapi/linux/virtio_mem.h
> @@ -185,10 +185,10 @@ struct virtio_mem_resp {
>
> struct virtio_mem_config {
> /* Block size and alignment. Cannot change. */
> - __u32 block_size;
> + __u64 block_size;
> /* Valid with VIRTIO_MEM_F_ACPI_PXM. Cannot change. */
> __u16 node_id;
> - __u16 padding;
> + __u8 padding[6];
> /* Start address of the memory region. Cannot change. */
> __u64 addr;
> /* Region size (maximum). Cannot change. */
>
--
Thanks,
David / dhildenb
On Mon, Jun 08, 2020 at 08:58:31AM +0200, David Hildenbrand wrote:
> On 08.06.20 08:14, Michael S. Tsirkin wrote:
> > If subblock size is large (e.g. 1G) 32 bit math involving it
> > can overflow. Rather than try to catch all instances of that,
> > let's tweak block size to 64 bit.
>
> I fail to see where we could actually trigger an overflow. The reported
> warning looked like a false positive to me.
So
const uint64_t size = count * vm->subblock_size;
is it unreasonable for count to be 4K with subblock_size being 1M?
> >
> > It ripples through UAPI which is an ABI change, but it's not too late to
> > make it, and it will allow supporting >4Gbyte blocks while might
> > become necessary down the road.
> >
>
> This might break cloud-hypervisor, who's already implementing this
> protocol upstream (ccing Hui).
> https://github.com/cloud-hypervisor/cloud-hypervisor/blob/master/vm-virtio/src/mem.rs
>
> (blocks in the gigabyte range were never the original intention of
> virtio-mem, but I am not completely opposed to that)
So in that case, can you code up validation in the probe function?
> > Fixes: 5f1f79bbc9e26 ("virtio-mem: Paravirtualized memory hotplug")
> > Signed-off-by: Michael S. Tsirkin <[email protected]>
> > ---
> > drivers/virtio/virtio_mem.c | 14 +++++++-------
> > include/uapi/linux/virtio_mem.h | 4 ++--
> > 2 files changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
> > index 2f357142ea5e..7b1bece8a331 100644
> > --- a/drivers/virtio/virtio_mem.c
> > +++ b/drivers/virtio/virtio_mem.c
> > @@ -77,7 +77,7 @@ struct virtio_mem {
> > uint64_t requested_size;
> >
> > /* The device block size (for communicating with the device). */
> > - uint32_t device_block_size;
> > + uint64_t device_block_size;
> > /* The translated node id. NUMA_NO_NODE in case not specified. */
> > int nid;
> > /* Physical start address of the memory region. */
> > @@ -86,7 +86,7 @@ struct virtio_mem {
> > uint64_t region_size;
> >
> > /* The subblock size. */
> > - uint32_t subblock_size;
> > + uint64_t subblock_size;
> > /* The number of subblocks per memory block. */
> > uint32_t nb_sb_per_mb;
> >
> > @@ -1698,9 +1698,9 @@ static int virtio_mem_init(struct virtio_mem *vm)
> > * - At least the device block size.
> > * In the worst case, a single subblock per memory block.
> > */
> > - vm->subblock_size = PAGE_SIZE * 1u << max_t(uint32_t, MAX_ORDER - 1,
> > - pageblock_order);
> > - vm->subblock_size = max_t(uint32_t, vm->device_block_size,
> > + vm->subblock_size = PAGE_SIZE * 1ul << max_t(uint32_t, MAX_ORDER - 1,
> > + pageblock_order);
> > + vm->subblock_size = max_t(uint64_t, vm->device_block_size,
> > vm->subblock_size);
> > vm->nb_sb_per_mb = memory_block_size_bytes() / vm->subblock_size;
> >
> > @@ -1713,8 +1713,8 @@ static int virtio_mem_init(struct virtio_mem *vm)
> >
> > dev_info(&vm->vdev->dev, "start address: 0x%llx", vm->addr);
> > dev_info(&vm->vdev->dev, "region size: 0x%llx", vm->region_size);
> > - dev_info(&vm->vdev->dev, "device block size: 0x%x",
> > - vm->device_block_size);
> > + dev_info(&vm->vdev->dev, "device block size: 0x%llx",
> > + (unsigned long long)vm->device_block_size);
> > dev_info(&vm->vdev->dev, "memory block size: 0x%lx",
> > memory_block_size_bytes());
> > dev_info(&vm->vdev->dev, "subblock size: 0x%x",
> > diff --git a/include/uapi/linux/virtio_mem.h b/include/uapi/linux/virtio_mem.h
> > index a455c488a995..a9ffe041843c 100644
> > --- a/include/uapi/linux/virtio_mem.h
> > +++ b/include/uapi/linux/virtio_mem.h
> > @@ -185,10 +185,10 @@ struct virtio_mem_resp {
> >
> > struct virtio_mem_config {
> > /* Block size and alignment. Cannot change. */
> > - __u32 block_size;
> > + __u64 block_size;
> > /* Valid with VIRTIO_MEM_F_ACPI_PXM. Cannot change. */
> > __u16 node_id;
> > - __u16 padding;
> > + __u8 padding[6];
> > /* Start address of the memory region. Cannot change. */
> > __u64 addr;
> > /* Region size (maximum). Cannot change. */
> >
>
>
> --
> Thanks,
>
> David / dhildenb
> 2020年6月8日 14:58，David Hildenbrand <[email protected]> 写道：
>
> On 08.06.20 08:14, Michael S. Tsirkin wrote:
>> If subblock size is large (e.g. 1G) 32 bit math involving it
>> can overflow. Rather than try to catch all instances of that,
>> let's tweak block size to 64 bit.
>
> I fail to see where we could actually trigger an overflow. The reported
> warning looked like a false positive to me.
>
>>
>> It ripples through UAPI which is an ABI change, but it's not too late to
>> make it, and it will allow supporting >4Gbyte blocks while might
>> become necessary down the road.
>>
>
> This might break cloud-hypervisor, who's already implementing this
> protocol upstream (ccing Hui).
> https://github.com/cloud-hypervisor/cloud-hypervisor/blob/master/vm-virtio/src/mem.rs
>
> (blocks in the gigabyte range were never the original intention of
> virtio-mem, but I am not completely opposed to that)
If you think virtio_mem need this patch, I think cloud-hypervisor should follow this update (I will post PR for it).
Best,
Hui
>
>> Fixes: 5f1f79bbc9e26 ("virtio-mem: Paravirtualized memory hotplug")
>> Signed-off-by: Michael S. Tsirkin <[email protected]>
>> ---
>> drivers/virtio/virtio_mem.c | 14 +++++++-------
>> include/uapi/linux/virtio_mem.h | 4 ++--
>> 2 files changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
>> index 2f357142ea5e..7b1bece8a331 100644
>> --- a/drivers/virtio/virtio_mem.c
>> +++ b/drivers/virtio/virtio_mem.c
>> @@ -77,7 +77,7 @@ struct virtio_mem {
>> uint64_t requested_size;
>>
>> /* The device block size (for communicating with the device). */
>> - uint32_t device_block_size;
>> + uint64_t device_block_size;
>> /* The translated node id. NUMA_NO_NODE in case not specified. */
>> int nid;
>> /* Physical start address of the memory region. */
>> @@ -86,7 +86,7 @@ struct virtio_mem {
>> uint64_t region_size;
>>
>> /* The subblock size. */
>> - uint32_t subblock_size;
>> + uint64_t subblock_size;
>> /* The number of subblocks per memory block. */
>> uint32_t nb_sb_per_mb;
>>
>> @@ -1698,9 +1698,9 @@ static int virtio_mem_init(struct virtio_mem *vm)
>> * - At least the device block size.
>> * In the worst case, a single subblock per memory block.
>> */
>> - vm->subblock_size = PAGE_SIZE * 1u << max_t(uint32_t, MAX_ORDER - 1,
>> - pageblock_order);
>> - vm->subblock_size = max_t(uint32_t, vm->device_block_size,
>> + vm->subblock_size = PAGE_SIZE * 1ul << max_t(uint32_t, MAX_ORDER - 1,
>> + pageblock_order);
>> + vm->subblock_size = max_t(uint64_t, vm->device_block_size,
>> vm->subblock_size);
>> vm->nb_sb_per_mb = memory_block_size_bytes() / vm->subblock_size;
>>
>> @@ -1713,8 +1713,8 @@ static int virtio_mem_init(struct virtio_mem *vm)
>>
>> dev_info(&vm->vdev->dev, "start address: 0x%llx", vm->addr);
>> dev_info(&vm->vdev->dev, "region size: 0x%llx", vm->region_size);
>> - dev_info(&vm->vdev->dev, "device block size: 0x%x",
>> - vm->device_block_size);
>> + dev_info(&vm->vdev->dev, "device block size: 0x%llx",
>> + (unsigned long long)vm->device_block_size);
>> dev_info(&vm->vdev->dev, "memory block size: 0x%lx",
>> memory_block_size_bytes());
>> dev_info(&vm->vdev->dev, "subblock size: 0x%x",
>> diff --git a/include/uapi/linux/virtio_mem.h b/include/uapi/linux/virtio_mem.h
>> index a455c488a995..a9ffe041843c 100644
>> --- a/include/uapi/linux/virtio_mem.h
>> +++ b/include/uapi/linux/virtio_mem.h
>> @@ -185,10 +185,10 @@ struct virtio_mem_resp {
>>
>> struct virtio_mem_config {
>> /* Block size and alignment. Cannot change. */
>> - __u32 block_size;
>> + __u64 block_size;
>> /* Valid with VIRTIO_MEM_F_ACPI_PXM. Cannot change. */
>> __u16 node_id;
>> - __u16 padding;
>> + __u8 padding[6];
>> /* Start address of the memory region. Cannot change. */
>> __u64 addr;
>> /* Region size (maximum). Cannot change. */
>>
>
>
> --
> Thanks,
>
> David / dhildenb
On 08.06.20 09:08, Michael S. Tsirkin wrote:
> On Mon, Jun 08, 2020 at 08:58:31AM +0200, David Hildenbrand wrote:
>> On 08.06.20 08:14, Michael S. Tsirkin wrote:
>>> If subblock size is large (e.g. 1G) 32 bit math involving it
>>> can overflow. Rather than try to catch all instances of that,
>>> let's tweak block size to 64 bit.
>>
>> I fail to see where we could actually trigger an overflow. The reported
>> warning looked like a false positive to me.
>
>
> So
>
> const uint64_t size = count * vm->subblock_size;
>
> is it unreasonable for count to be 4K with subblock_size being 1M?
virtio_mem_mb_plug_sb() and friends are only called on subblocks
residing within a single Linux memory block. (currently, 128MB .. 2G on
x86-64). A subblock on x86-64 is currently at least 4MB.
So "count * vm->subblock_size" can currently not exceed the Linux memory
block size (in practice, it is max 128MB).
>
>>>
>>> It ripples through UAPI which is an ABI change, but it's not too late to
>>> make it, and it will allow supporting >4Gbyte blocks while might
>>> become necessary down the road.
>>>
>>
>> This might break cloud-hypervisor, who's already implementing this
>> protocol upstream (ccing Hui).
>> https://github.com/cloud-hypervisor/cloud-hypervisor/blob/master/vm-virtio/src/mem.rs
>>
>> (blocks in the gigabyte range were never the original intention of
>> virtio-mem, but I am not completely opposed to that)
>
>
> So in that case, can you code up validation in the probe function?
If we would currently have a "block_size" > Linux memory block size, we
bail out.
virtio_mem_init():
if (vm->device_block_size > memory_block_size_bytes()) {
dev_err(&vm->vdev->dev,
"The block size is not supported (too big).\n");
return -EINVAL;
}
So what's reported can currently not happen. Having that said, changing
"subblock_size" to be an uint64_t is a good cleanup, especially for the
future.
--
Thanks,
David / dhildenb
On 08.06.20 09:12, teawater wrote:
>
>
>> 2020年6月8日 14:58，David Hildenbrand <[email protected]> 写道：
>>
>> On 08.06.20 08:14, Michael S. Tsirkin wrote:
>>> If subblock size is large (e.g. 1G) 32 bit math involving it
>>> can overflow. Rather than try to catch all instances of that,
>>> let's tweak block size to 64 bit.
>>
>> I fail to see where we could actually trigger an overflow. The reported
>> warning looked like a false positive to me.
>>
>>>
>>> It ripples through UAPI which is an ABI change, but it's not too late to
>>> make it, and it will allow supporting >4Gbyte blocks while might
>>> become necessary down the road.
>>>
>>
>> This might break cloud-hypervisor, who's already implementing this
>> protocol upstream (ccing Hui).
>> https://github.com/cloud-hypervisor/cloud-hypervisor/blob/master/vm-virtio/src/mem.rs
>>
>> (blocks in the gigabyte range were never the original intention of
>> virtio-mem, but I am not completely opposed to that)
>
> If you think virtio_mem need this patch, I think cloud-hypervisor should follow this update (I will post PR for it).
Thanks Hui. So we can still do last-minute changes if we all agree it
makes sense.
@MST can you rephrase the patch description to highlight that this is a
preparation for the future only and not actually currently broken?
"virtio-mem: convert device block size into 64bit" ...
With that
Acked-by: David Hildenbrand <[email protected]>
--
Thanks,
David / dhildenb
On Mon, Jun 08, 2020 at 09:17:45AM +0200, David Hildenbrand wrote:
> On 08.06.20 09:08, Michael S. Tsirkin wrote:
> > On Mon, Jun 08, 2020 at 08:58:31AM +0200, David Hildenbrand wrote:
> >> On 08.06.20 08:14, Michael S. Tsirkin wrote:
> >>> If subblock size is large (e.g. 1G) 32 bit math involving it
> >>> can overflow. Rather than try to catch all instances of that,
> >>> let's tweak block size to 64 bit.
> >>
> >> I fail to see where we could actually trigger an overflow. The reported
> >> warning looked like a false positive to me.
> >
> >
> > So
> >
> > const uint64_t size = count * vm->subblock_size;
> >
> > is it unreasonable for count to be 4K with subblock_size being 1M?
>
> virtio_mem_mb_plug_sb() and friends are only called on subblocks
> residing within a single Linux memory block. (currently, 128MB .. 2G on
> x86-64). A subblock on x86-64 is currently at least 4MB.
>
> So "count * vm->subblock_size" can currently not exceed the Linux memory
> block size (in practice, it is max 128MB).
>
> >
> >>>
> >>> It ripples through UAPI which is an ABI change, but it's not too late to
> >>> make it, and it will allow supporting >4Gbyte blocks while might
> >>> become necessary down the road.
> >>>
> >>
> >> This might break cloud-hypervisor, who's already implementing this
> >> protocol upstream (ccing Hui).
> >> https://github.com/cloud-hypervisor/cloud-hypervisor/blob/master/vm-virtio/src/mem.rs
> >>
> >> (blocks in the gigabyte range were never the original intention of
> >> virtio-mem, but I am not completely opposed to that)
> >
> >
> > So in that case, can you code up validation in the probe function?
>
> If we would currently have a "block_size" > Linux memory block size, we
> bail out.
>
> virtio_mem_init():
>
> if (vm->device_block_size > memory_block_size_bytes()) {
> dev_err(&vm->vdev->dev,
> "The block size is not supported (too big).\n");
> return -EINVAL;
> }
Sounds good.
> So what's reported can currently not happen. Having that said, changing
> "subblock_size" to be an uint64_t is a good cleanup, especially for the
> future.
OK, no need to argue about it then. I tweaked the subject as you
suggested and queued it then.
>
>
>
> --
> Thanks,
>
> David / dhildenb
Hi "Michael,
I love your patch! Perhaps something to improve:
[auto build test WARNING on vhost/vhost]
[also build test WARNING on next-20200608]
[cannot apply to linus/master linux/master v5.7]
[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/Michael-S-Tsirkin/virtio_mem-prevent-overflow-with-subblock-size/20200608-141805
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project e429cffd4f228f70c1d9df0e5d77c08590dd9766)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>, old ones prefixed by <<):
>> drivers/virtio/virtio_mem.c:1721:4: warning: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long long') [-Wformat]
vm->subblock_size);
^~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:33: note: expanded from macro 'dev_info'
_dev_info(dev, dev_fmt(fmt), ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
1 warning generated.
vim +1721 drivers/virtio/virtio_mem.c
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1642
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1643 static int virtio_mem_init(struct virtio_mem *vm)
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1644 {
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1645 const uint64_t phys_limit = 1UL << MAX_PHYSMEM_BITS;
f2af6d3978d74a David Hildenbrand 2020-05-07 1646 uint16_t node_id;
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1647
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1648 if (!vm->vdev->config->get) {
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1649 dev_err(&vm->vdev->dev, "config access disabled\n");
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1650 return -EINVAL;
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1651 }
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1652
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1653 /*
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1654 * We don't want to (un)plug or reuse any memory when in kdump. The
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1655 * memory is still accessible (but not mapped).
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1656 */
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1657 if (is_kdump_kernel()) {
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1658 dev_warn(&vm->vdev->dev, "disabled in kdump kernel\n");
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1659 return -EBUSY;
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1660 }
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1661
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1662 /* Fetch all properties that can't change. */
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1663 virtio_cread(vm->vdev, struct virtio_mem_config, plugged_size,
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1664 &vm->plugged_size);
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1665 virtio_cread(vm->vdev, struct virtio_mem_config, block_size,
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1666 &vm->device_block_size);
f2af6d3978d74a David Hildenbrand 2020-05-07 1667 virtio_cread(vm->vdev, struct virtio_mem_config, node_id,
f2af6d3978d74a David Hildenbrand 2020-05-07 1668 &node_id);
f2af6d3978d74a David Hildenbrand 2020-05-07 1669 vm->nid = virtio_mem_translate_node_id(vm, node_id);
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1670 virtio_cread(vm->vdev, struct virtio_mem_config, addr, &vm->addr);
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1671 virtio_cread(vm->vdev, struct virtio_mem_config, region_size,
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1672 &vm->region_size);
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1673
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1674 /*
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1675 * We always hotplug memory in memory block granularity. This way,
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1676 * we have to wait for exactly one memory block to online.
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1677 */
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1678 if (vm->device_block_size > memory_block_size_bytes()) {
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1679 dev_err(&vm->vdev->dev,
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1680 "The block size is not supported (too big).\n");
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1681 return -EINVAL;
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1682 }
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1683
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1684 /* bad device setup - warn only */
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1685 if (!IS_ALIGNED(vm->addr, memory_block_size_bytes()))
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1686 dev_warn(&vm->vdev->dev,
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1687 "The alignment of the physical start address can make some memory unusable.\n");
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1688 if (!IS_ALIGNED(vm->addr + vm->region_size, memory_block_size_bytes()))
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1689 dev_warn(&vm->vdev->dev,
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1690 "The alignment of the physical end address can make some memory unusable.\n");
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1691 if (vm->addr + vm->region_size > phys_limit)
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1692 dev_warn(&vm->vdev->dev,
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1693 "Some memory is not addressable. This can make some memory unusable.\n");
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1694
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1695 /*
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1696 * Calculate the subblock size:
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1697 * - At least MAX_ORDER - 1 / pageblock_order.
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1698 * - At least the device block size.
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1699 * In the worst case, a single subblock per memory block.
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1700 */
09760a643f90c4 Michael S. Tsirkin 2020-06-08 1701 vm->subblock_size = PAGE_SIZE * 1ul << max_t(uint32_t, MAX_ORDER - 1,
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1702 pageblock_order);
09760a643f90c4 Michael S. Tsirkin 2020-06-08 1703 vm->subblock_size = max_t(uint64_t, vm->device_block_size,
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1704 vm->subblock_size);
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1705 vm->nb_sb_per_mb = memory_block_size_bytes() / vm->subblock_size;
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1706
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1707 /* Round up to the next full memory block */
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1708 vm->first_mb_id = virtio_mem_phys_to_mb_id(vm->addr - 1 +
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1709 memory_block_size_bytes());
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1710 vm->next_mb_id = vm->first_mb_id;
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1711 vm->last_mb_id = virtio_mem_phys_to_mb_id(vm->addr +
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1712 vm->region_size) - 1;
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1713
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1714 dev_info(&vm->vdev->dev, "start address: 0x%llx", vm->addr);
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1715 dev_info(&vm->vdev->dev, "region size: 0x%llx", vm->region_size);
09760a643f90c4 Michael S. Tsirkin 2020-06-08 1716 dev_info(&vm->vdev->dev, "device block size: 0x%llx",
09760a643f90c4 Michael S. Tsirkin 2020-06-08 1717 (unsigned long long)vm->device_block_size);
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1718 dev_info(&vm->vdev->dev, "memory block size: 0x%lx",
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1719 memory_block_size_bytes());
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1720 dev_info(&vm->vdev->dev, "subblock size: 0x%x",
5f1f79bbc9e26f David Hildenbrand 2020-05-07 @1721 vm->subblock_size);
f2af6d3978d74a David Hildenbrand 2020-05-07 1722 if (vm->nid != NUMA_NO_NODE)
f2af6d3978d74a David Hildenbrand 2020-05-07 1723 dev_info(&vm->vdev->dev, "nid: %d", vm->nid);
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1724
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1725 return 0;
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1726 }
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1727
:::::: The code at line 1721 was first introduced by commit
:::::: 5f1f79bbc9e26fa9412fa9522f957bb8f030c442 virtio-mem: Paravirtualized memory hotplug
:::::: TO: David Hildenbrand <[email protected]>
:::::: CC: Michael S. Tsirkin <[email protected]>
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]