Re: [PATCH v2] hw/arm/virt enable support for virtio-mem

2020-12-02 Thread Jonathan Cameron
On Wed, 2 Dec 2020 05:02:57 -0500
"Michael S. Tsirkin"  wrote:

> On Wed, Nov 25, 2020 at 02:56:59PM +, Jonathan Cameron wrote:
> > Cool.  I'll run a few more comprehensive tests then send out the
> > trivial patch to enable the kernel option + v2 of the qemu support.  
> 
> IIUC there will be another version of this patch, right?
> 

Yes.  Busy period so might be a little while yet to complete testing
before v2.

Jonathan
 



Re: [PATCH v2] hw/arm/virt enable support for virtio-mem

2020-12-02 Thread Michael S. Tsirkin
On Wed, Nov 25, 2020 at 02:56:59PM +, Jonathan Cameron wrote:
> Cool.  I'll run a few more comprehensive tests then send out the
> trivial patch to enable the kernel option + v2 of the qemu support.

IIUC there will be another version of this patch, right?

-- 
MST




Re: [PATCH v2] hw/arm/virt enable support for virtio-mem

2020-11-25 Thread David Hildenbrand
On 25.11.20 17:11, Jonathan Cameron wrote:
> On Wed, 25 Nov 2020 16:54:53 +0100
> David Hildenbrand  wrote:
> 
>>
>> 64k guest on 4k host with 512MiB block size seems fine.
>>
>> If there are any places anyone thinks need particular poking I'd 
>> appreciate a hint :)
>
> If things seem to work for now, that's great :) Thanks!
>  
 Cool.  I'll run a few more comprehensive tests then send out the
 trivial patch to enable the kernel option + v2 of the qemu support.  
>>>
>>> Perfect, thanks!  
>>
>> Oh, btw, I have no idea what the state of vfio-pci + QEMU on arm64 is.
>> In case it's supposed to work, you could give
>>
>> https://lkml.kernel.org/r/20201119153918.120976-1-da...@redhat.com
>>
>> to see what we're missing.
> 
> vfio-pci works in general (and we use it a lot), so sure I'll give
> this a test run.

Cool.

In case you get it to run, please test with both "online_kernel" and
"online_movable" in the guest, and small boot memory (e.g., 2 GiB).

For example, on x86-64 I got my vfio-pci provided GPUs to consume
virtio-mem memory easily when starting with 2-4 GiB boot memory and
using "online_kernel". (I verified that when not creating the mappings,
IO errors can be observed and graphics are messed up).

-- 
Thanks,

David / dhildenb




Re: [PATCH v2] hw/arm/virt enable support for virtio-mem

2020-11-25 Thread Jonathan Cameron
On Wed, 25 Nov 2020 16:54:53 +0100
David Hildenbrand  wrote:

> 
>  64k guest on 4k host with 512MiB block size seems fine.
> 
>  If there are any places anyone thinks need particular poking I'd 
>  appreciate a hint :)
> >>>
> >>> If things seem to work for now, that's great :) Thanks!
> >>>  
> >> Cool.  I'll run a few more comprehensive tests then send out the
> >> trivial patch to enable the kernel option + v2 of the qemu support.  
> > 
> > Perfect, thanks!  
> 
> Oh, btw, I have no idea what the state of vfio-pci + QEMU on arm64 is.
> In case it's supposed to work, you could give
> 
> https://lkml.kernel.org/r/20201119153918.120976-1-da...@redhat.com
> 
> to see what we're missing.

vfio-pci works in general (and we use it a lot), so sure I'll give
this a test run.

> 
> I added a short virtio-pci guide to
> 
> https://virtio-mem.gitlab.io/user-guide/user-guide-qemu.html
> 

Thanks,

Jonathan



Re: [PATCH v2] hw/arm/virt enable support for virtio-mem

2020-11-25 Thread David Hildenbrand

 64k guest on 4k host with 512MiB block size seems fine.

 If there are any places anyone thinks need particular poking I'd 
 appreciate a hint :)  
>>>
>>> If things seem to work for now, that's great :) Thanks!
>>>
>> Cool.  I'll run a few more comprehensive tests then send out the
>> trivial patch to enable the kernel option + v2 of the qemu support.
> 
> Perfect, thanks!

Oh, btw, I have no idea what the state of vfio-pci + QEMU on arm64 is.
In case it's supposed to work, you could give

https://lkml.kernel.org/r/20201119153918.120976-1-da...@redhat.com

to see what we're missing.

I added a short virtio-pci guide to

https://virtio-mem.gitlab.io/user-guide/user-guide-qemu.html

-- 
Thanks,

David / dhildenb




Re: [PATCH v2] hw/arm/virt enable support for virtio-mem

2020-11-25 Thread David Hildenbrand
> Ah. I'd missed that quirk around MAX_ORDER.  It's also true of ARM64 with
> 4k pages.  As you can probably guess I'd forgotten to recompile my 4k test
> kernel after adding that particular check. :(
> 
> Ah well. Given we are already in a situation where adding 2MiB doesn't 
> actually
> do anything useful, I guess it's not really a problem to merrily let the host
> add less than the guest can make use of.  So we allow adding any multiple of
> 2MiB but reality is the guest isn't going to use anything other than 512MiB
> chunks.

Right, and the host can observe the change not happening when not
aligned to 512 MB. There are plans for a virtio-mem extension for the
guest to present a status - e.g., why the requested size cannot be
achieved (requested size not alignment, usable region too small,
ENOMEM/EBUSY when unplugging, ...).

[...]

>>>
>>> 4K guest on 64K host seems fine and no such limit is needed - though there
>>> may be performance issues doing that.  
>>
>> Yeah, if one is lucky to get one of these 512 MiB huge pages at all :)
> 
> Not too hard on my 1TB test system that's running nothing much else, but 
> agreed it
> won't be trivial more generally.

Hehe, right ! (... and here I am, testing with 64GB machines ... :) )

It's more of an issue in the guest to get 512 MB without ZONE_MOVABLE to
unplug ... especially with smaller VMs.

> 
>>
>>>
>>> 64k guest on 4k host with 512MiB block size seems fine.
>>>
>>> If there are any places anyone thinks need particular poking I'd appreciate 
>>> a hint :)  
>>
>> If things seem to work for now, that's great :) Thanks!
>>
> Cool.  I'll run a few more comprehensive tests then send out the
> trivial patch to enable the kernel option + v2 of the qemu support.

Perfect, thanks!

-- 
Thanks,

David / dhildenb




Re: [PATCH v2] hw/arm/virt enable support for virtio-mem

2020-11-25 Thread Jonathan Cameron
On Tue, 24 Nov 2020 20:17:35 +0100
David Hildenbrand  wrote:

> On 24.11.20 19:11, Jonathan Cameron wrote:
> > On Mon, 9 Nov 2020 20:47:09 +0100
> > David Hildenbrand  wrote:
> > 
> > +CC Eric based on similar query in other branch of the thread.
> >   
> >> On 05.11.20 18:43, Jonathan Cameron wrote:  
> >>> Basically a cut and paste job from the x86 support with the exception of
> >>> needing a larger block size as the Memory Block Size (MIN_SECTION_SIZE)
> >>> on ARM64 in Linux is 1G.
> >>>
> >>> Tested:
> >>> * In full emulation and with KVM on an arm64 server.
> >>> * cold and hotplug for the virtio-mem-pci device.
> >>> * Wide range of memory sizes, added at creation and later.
> >>> * Fairly basic memory usage of memory added.  Seems to function as normal.
> >>> * NUMA setup with virtio-mem-pci devices on each node.
> >>> * Simple migration test.
> >>>
> >>> Related kernel patch just enables the Kconfig item for ARM64 as an
> >>> alternative to x86 in drivers/virtio/Kconfig
> >>>
> >>> The original patches from David Hildenbrand stated that he thought it 
> >>> should
> >>> work for ARM64 but it wasn't enabled in the kernel [1]
> >>> It appears he was correct and everything 'just works'.
> >>>
> >>> The build system related stuff is intended to ensure virtio-mem support is
> >>> not built for arm32 (build will fail due no defined block size).
> >>> If there is a more elegant way to do this, please point me in the right
> >>> direction.
> >>
> >> You might be aware of https://virtio-mem.gitlab.io/developer-guide.html 
> >> and the "issue" with 64k base pages - 512MB granularity. Similar as the 
> >> question from Auger, have you tried running arm64 with differing page 
> >> sizes in host/guest?
> >>  
> > 
> > Hi David,
> >   
> >> With recent kernels, you can use "memhp_default_state=online_movable" on 
> >> the kernel cmdline to make memory unplug more likely to succeed - 
> >> especially with 64k base pages. You just have to be sure to not hotplug 
> >> "too much memory" to a VM.  
> > 
> > Thanks for the pointer - that definitely simplifies testing.  Was getting a 
> > bit
> > tedious with out that.
> > 
> > As ever other stuff got in the way, so I only just got back to looking at 
> > this.
> > 
> > I've not done a particularly comprehensive set of tests yet, but things seem
> > to 'work' with mixed page sizes.
> > 
> > With 64K pages in general, you run into a problem with the device 
> > block_size being
> > smaller than the subblock_size.  I've just added a check for that into the  
> 
> "device block size smaller than subblock size" - that's very common,
> e.g.,  on x86-64.
> 
> E.g., device_block_size is 2MiB, subblock size 4MiB - until we improve
> that in the future in Linux guests.

Ah. I'd missed that quirk around MAX_ORDER.  It's also true of ARM64 with
4k pages.  As you can probably guess I'd forgotten to recompile my 4k test
kernel after adding that particular check. :(

Ah well. Given we are already in a situation where adding 2MiB doesn't actually
do anything useful, I guess it's not really a problem to merrily let the host
add less than the guest can make use of.  So we allow adding any multiple of
2MiB but reality is the guest isn't going to use anything other than 512MiB
chunks.

> 
> Or did you mean something else?
> 
> > virtio-mem kernel driver and have it fail to probe if that happens.  I don't
> > think such a setup makes any sense anyway so no loss there.  Should it make 
> > sense
> > to drop that restriction in the future we can deal with that then without 
> > breaking
> > backwards compatibility.
> > 
> > So the question is whether it makes sense to bother with virtio-mem support
> > at all on ARM64 with 64k pages given currently the minimum workable 
> > block_size
> > is 512MiB?  I guess there is an argument of virtio-mem being a possibly more
> > convenient interface than full memory HP.  Curious to hear what people 
> > think on
> > this?  
> 
> IMHO we really want it. For example, RHEL is always 64k. This is a
> current guest limitation, to be improved in the future - either by
> moving away from 512MB huge pages with 64k or by improving
> alloc_contig_range().

Sure.  Would be great if we do one day support 2MiB on 64k.

> 
> > 
> > 4K guest on 64K host seems fine and no such limit is needed - though there
> > may be performance issues doing that.  
> 
> Yeah, if one is lucky to get one of these 512 MiB huge pages at all :)

Not too hard on my 1TB test system that's running nothing much else, but agreed 
it
won't be trivial more generally.

> 
> > 
> > 64k guest on 4k host with 512MiB block size seems fine.
> > 
> > If there are any places anyone thinks need particular poking I'd appreciate 
> > a hint :)  
> 
> If things seem to work for now, that's great :) Thanks!
> 
Cool.  I'll run a few more comprehensive tests then send out the
trivial patch to enable the kernel option + v2 of the qemu support.

Jonathan




Re: [PATCH v2] hw/arm/virt enable support for virtio-mem

2020-11-25 Thread David Hildenbrand
On 25.11.20 11:47, Andrew Jones wrote:
> On Wed, Nov 25, 2020 at 09:45:19AM +0100, David Hildenbrand wrote:
>> On 25.11.20 09:38, Andrew Jones wrote:
>>> On Tue, Nov 24, 2020 at 08:17:35PM +0100, David Hildenbrand wrote:
 On 24.11.20 19:11, Jonathan Cameron wrote:
> On Mon, 9 Nov 2020 20:47:09 +0100
> David Hildenbrand  wrote:
>
> +CC Eric based on similar query in other branch of the thread.
>
>> On 05.11.20 18:43, Jonathan Cameron wrote:
>>> Basically a cut and paste job from the x86 support with the exception of
>>> needing a larger block size as the Memory Block Size (MIN_SECTION_SIZE)
>>> on ARM64 in Linux is 1G.
>>>
>>> Tested:
>>> * In full emulation and with KVM on an arm64 server.
>>> * cold and hotplug for the virtio-mem-pci device.
>>> * Wide range of memory sizes, added at creation and later.
>>> * Fairly basic memory usage of memory added.  Seems to function as 
>>> normal.
>>> * NUMA setup with virtio-mem-pci devices on each node.
>>> * Simple migration test.
>>>
>>> Related kernel patch just enables the Kconfig item for ARM64 as an
>>> alternative to x86 in drivers/virtio/Kconfig
>>>
>>> The original patches from David Hildenbrand stated that he thought it 
>>> should
>>> work for ARM64 but it wasn't enabled in the kernel [1]
>>> It appears he was correct and everything 'just works'.
>>>
>>> The build system related stuff is intended to ensure virtio-mem support 
>>> is
>>> not built for arm32 (build will fail due no defined block size).
>>> If there is a more elegant way to do this, please point me in the right
>>> direction.  
>>
>> You might be aware of https://virtio-mem.gitlab.io/developer-guide.html 
>> and the "issue" with 64k base pages - 512MB granularity. Similar as the 
>> question from Auger, have you tried running arm64 with differing page 
>> sizes in host/guest?
>>
>
> Hi David,
>
>> With recent kernels, you can use "memhp_default_state=online_movable" on 
>> the kernel cmdline to make memory unplug more likely to succeed - 
>> especially with 64k base pages. You just have to be sure to not hotplug 
>> "too much memory" to a VM.
>
> Thanks for the pointer - that definitely simplifies testing.  Was getting 
> a bit
> tedious with out that.
>
> As ever other stuff got in the way, so I only just got back to looking at 
> this.
>
> I've not done a particularly comprehensive set of tests yet, but things 
> seem
> to 'work' with mixed page sizes.
>
> With 64K pages in general, you run into a problem with the device 
> block_size being
> smaller than the subblock_size.  I've just added a check for that into the

 "device block size smaller than subblock size" - that's very common,
 e.g.,  on x86-64.

 E.g., device_block_size is 2MiB, subblock size 4MiB - until we improve
 that in the future in Linux guests.

 Or did you mean something else?

> virtio-mem kernel driver and have it fail to probe if that happens.  I 
> don't
> think such a setup makes any sense anyway so no loss there.  Should it 
> make sense
> to drop that restriction in the future we can deal with that then without 
> breaking
> backwards compatibility.
>
> So the question is whether it makes sense to bother with virtio-mem 
> support
> at all on ARM64 with 64k pages given currently the minimum workable 
> block_size
> is 512MiB?  I guess there is an argument of virtio-mem being a possibly 
> more
> convenient interface than full memory HP.  Curious to hear what people 
> think on
> this?

 IMHO we really want it. For example, RHEL is always 64k. This is a
 current guest limitation, to be improved in the future - either by
 moving away from 512MB huge pages with 64k or by improving
 alloc_contig_range().
>>>
>>> Even with 64k pages you may be able to have 2MB huge pages by setting
>>> default_hugepagesz=2M on the kernel command line.
>>
>> Yes, but not for THP, right? Last time I checked that move was not
>> performed yet - resulting in MAX_ORDER/pageblock_order in Linux
>> corresponding to 512 MB.
>>
> 
> Yes, I believe you're correct. At least on the machine I've booted with
> default_hugepagesz=2M, I see
> 
>  $ cat /sys/kernel/mm/transparent_hugepage/hpage_pmd_size
>  536870912
> 
> (I'm not running a latest mainline kernel though.)

I remember some upstream discussions where people raised that switching
to 2 MB THP might be possible (implemented via cont bits in the
pagetables - similar to 2MB huge pages you mentioned). 512 MB really
sounds more like gigantic pages after all.

-- 
Thanks,

David / dhildenb




Re: [PATCH v2] hw/arm/virt enable support for virtio-mem

2020-11-25 Thread Andrew Jones
On Wed, Nov 25, 2020 at 09:45:19AM +0100, David Hildenbrand wrote:
> On 25.11.20 09:38, Andrew Jones wrote:
> > On Tue, Nov 24, 2020 at 08:17:35PM +0100, David Hildenbrand wrote:
> >> On 24.11.20 19:11, Jonathan Cameron wrote:
> >>> On Mon, 9 Nov 2020 20:47:09 +0100
> >>> David Hildenbrand  wrote:
> >>>
> >>> +CC Eric based on similar query in other branch of the thread.
> >>>
>  On 05.11.20 18:43, Jonathan Cameron wrote:
> > Basically a cut and paste job from the x86 support with the exception of
> > needing a larger block size as the Memory Block Size (MIN_SECTION_SIZE)
> > on ARM64 in Linux is 1G.
> >
> > Tested:
> > * In full emulation and with KVM on an arm64 server.
> > * cold and hotplug for the virtio-mem-pci device.
> > * Wide range of memory sizes, added at creation and later.
> > * Fairly basic memory usage of memory added.  Seems to function as 
> > normal.
> > * NUMA setup with virtio-mem-pci devices on each node.
> > * Simple migration test.
> >
> > Related kernel patch just enables the Kconfig item for ARM64 as an
> > alternative to x86 in drivers/virtio/Kconfig
> >
> > The original patches from David Hildenbrand stated that he thought it 
> > should
> > work for ARM64 but it wasn't enabled in the kernel [1]
> > It appears he was correct and everything 'just works'.
> >
> > The build system related stuff is intended to ensure virtio-mem support 
> > is
> > not built for arm32 (build will fail due no defined block size).
> > If there is a more elegant way to do this, please point me in the right
> > direction.  
> 
>  You might be aware of https://virtio-mem.gitlab.io/developer-guide.html 
>  and the "issue" with 64k base pages - 512MB granularity. Similar as the 
>  question from Auger, have you tried running arm64 with differing page 
>  sizes in host/guest?
> 
> >>>
> >>> Hi David,
> >>>
>  With recent kernels, you can use "memhp_default_state=online_movable" on 
>  the kernel cmdline to make memory unplug more likely to succeed - 
>  especially with 64k base pages. You just have to be sure to not hotplug 
>  "too much memory" to a VM.
> >>>
> >>> Thanks for the pointer - that definitely simplifies testing.  Was getting 
> >>> a bit
> >>> tedious with out that.
> >>>
> >>> As ever other stuff got in the way, so I only just got back to looking at 
> >>> this.
> >>>
> >>> I've not done a particularly comprehensive set of tests yet, but things 
> >>> seem
> >>> to 'work' with mixed page sizes.
> >>>
> >>> With 64K pages in general, you run into a problem with the device 
> >>> block_size being
> >>> smaller than the subblock_size.  I've just added a check for that into the
> >>
> >> "device block size smaller than subblock size" - that's very common,
> >> e.g.,  on x86-64.
> >>
> >> E.g., device_block_size is 2MiB, subblock size 4MiB - until we improve
> >> that in the future in Linux guests.
> >>
> >> Or did you mean something else?
> >>
> >>> virtio-mem kernel driver and have it fail to probe if that happens.  I 
> >>> don't
> >>> think such a setup makes any sense anyway so no loss there.  Should it 
> >>> make sense
> >>> to drop that restriction in the future we can deal with that then without 
> >>> breaking
> >>> backwards compatibility.
> >>>
> >>> So the question is whether it makes sense to bother with virtio-mem 
> >>> support
> >>> at all on ARM64 with 64k pages given currently the minimum workable 
> >>> block_size
> >>> is 512MiB?  I guess there is an argument of virtio-mem being a possibly 
> >>> more
> >>> convenient interface than full memory HP.  Curious to hear what people 
> >>> think on
> >>> this?
> >>
> >> IMHO we really want it. For example, RHEL is always 64k. This is a
> >> current guest limitation, to be improved in the future - either by
> >> moving away from 512MB huge pages with 64k or by improving
> >> alloc_contig_range().
> > 
> > Even with 64k pages you may be able to have 2MB huge pages by setting
> > default_hugepagesz=2M on the kernel command line.
> 
> Yes, but not for THP, right? Last time I checked that move was not
> performed yet - resulting in MAX_ORDER/pageblock_order in Linux
> corresponding to 512 MB.
>

Yes, I believe you're correct. At least on the machine I've booted with
default_hugepagesz=2M, I see

 $ cat /sys/kernel/mm/transparent_hugepage/hpage_pmd_size
 536870912

(I'm not running a latest mainline kernel though.)

Thanks,
drew




Re: [PATCH v2] hw/arm/virt enable support for virtio-mem

2020-11-25 Thread David Hildenbrand
On 25.11.20 09:38, Andrew Jones wrote:
> On Tue, Nov 24, 2020 at 08:17:35PM +0100, David Hildenbrand wrote:
>> On 24.11.20 19:11, Jonathan Cameron wrote:
>>> On Mon, 9 Nov 2020 20:47:09 +0100
>>> David Hildenbrand  wrote:
>>>
>>> +CC Eric based on similar query in other branch of the thread.
>>>
 On 05.11.20 18:43, Jonathan Cameron wrote:
> Basically a cut and paste job from the x86 support with the exception of
> needing a larger block size as the Memory Block Size (MIN_SECTION_SIZE)
> on ARM64 in Linux is 1G.
>
> Tested:
> * In full emulation and with KVM on an arm64 server.
> * cold and hotplug for the virtio-mem-pci device.
> * Wide range of memory sizes, added at creation and later.
> * Fairly basic memory usage of memory added.  Seems to function as normal.
> * NUMA setup with virtio-mem-pci devices on each node.
> * Simple migration test.
>
> Related kernel patch just enables the Kconfig item for ARM64 as an
> alternative to x86 in drivers/virtio/Kconfig
>
> The original patches from David Hildenbrand stated that he thought it 
> should
> work for ARM64 but it wasn't enabled in the kernel [1]
> It appears he was correct and everything 'just works'.
>
> The build system related stuff is intended to ensure virtio-mem support is
> not built for arm32 (build will fail due no defined block size).
> If there is a more elegant way to do this, please point me in the right
> direction.  

 You might be aware of https://virtio-mem.gitlab.io/developer-guide.html 
 and the "issue" with 64k base pages - 512MB granularity. Similar as the 
 question from Auger, have you tried running arm64 with differing page 
 sizes in host/guest?

>>>
>>> Hi David,
>>>
 With recent kernels, you can use "memhp_default_state=online_movable" on 
 the kernel cmdline to make memory unplug more likely to succeed - 
 especially with 64k base pages. You just have to be sure to not hotplug 
 "too much memory" to a VM.
>>>
>>> Thanks for the pointer - that definitely simplifies testing.  Was getting a 
>>> bit
>>> tedious with out that.
>>>
>>> As ever other stuff got in the way, so I only just got back to looking at 
>>> this.
>>>
>>> I've not done a particularly comprehensive set of tests yet, but things seem
>>> to 'work' with mixed page sizes.
>>>
>>> With 64K pages in general, you run into a problem with the device 
>>> block_size being
>>> smaller than the subblock_size.  I've just added a check for that into the
>>
>> "device block size smaller than subblock size" - that's very common,
>> e.g.,  on x86-64.
>>
>> E.g., device_block_size is 2MiB, subblock size 4MiB - until we improve
>> that in the future in Linux guests.
>>
>> Or did you mean something else?
>>
>>> virtio-mem kernel driver and have it fail to probe if that happens.  I don't
>>> think such a setup makes any sense anyway so no loss there.  Should it make 
>>> sense
>>> to drop that restriction in the future we can deal with that then without 
>>> breaking
>>> backwards compatibility.
>>>
>>> So the question is whether it makes sense to bother with virtio-mem support
>>> at all on ARM64 with 64k pages given currently the minimum workable 
>>> block_size
>>> is 512MiB?  I guess there is an argument of virtio-mem being a possibly more
>>> convenient interface than full memory HP.  Curious to hear what people 
>>> think on
>>> this?
>>
>> IMHO we really want it. For example, RHEL is always 64k. This is a
>> current guest limitation, to be improved in the future - either by
>> moving away from 512MB huge pages with 64k or by improving
>> alloc_contig_range().
> 
> Even with 64k pages you may be able to have 2MB huge pages by setting
> default_hugepagesz=2M on the kernel command line.

Yes, but not for THP, right? Last time I checked that move was not
performed yet - resulting in MAX_ORDER/pageblock_order in Linux
corresponding to 512 MB.

> 
> Thanks,
> drew


-- 
Thanks,

David / dhildenb




Re: [PATCH v2] hw/arm/virt enable support for virtio-mem

2020-11-25 Thread Andrew Jones
On Tue, Nov 24, 2020 at 08:17:35PM +0100, David Hildenbrand wrote:
> On 24.11.20 19:11, Jonathan Cameron wrote:
> > On Mon, 9 Nov 2020 20:47:09 +0100
> > David Hildenbrand  wrote:
> > 
> > +CC Eric based on similar query in other branch of the thread.
> > 
> >> On 05.11.20 18:43, Jonathan Cameron wrote:
> >>> Basically a cut and paste job from the x86 support with the exception of
> >>> needing a larger block size as the Memory Block Size (MIN_SECTION_SIZE)
> >>> on ARM64 in Linux is 1G.
> >>>
> >>> Tested:
> >>> * In full emulation and with KVM on an arm64 server.
> >>> * cold and hotplug for the virtio-mem-pci device.
> >>> * Wide range of memory sizes, added at creation and later.
> >>> * Fairly basic memory usage of memory added.  Seems to function as normal.
> >>> * NUMA setup with virtio-mem-pci devices on each node.
> >>> * Simple migration test.
> >>>
> >>> Related kernel patch just enables the Kconfig item for ARM64 as an
> >>> alternative to x86 in drivers/virtio/Kconfig
> >>>
> >>> The original patches from David Hildenbrand stated that he thought it 
> >>> should
> >>> work for ARM64 but it wasn't enabled in the kernel [1]
> >>> It appears he was correct and everything 'just works'.
> >>>
> >>> The build system related stuff is intended to ensure virtio-mem support is
> >>> not built for arm32 (build will fail due no defined block size).
> >>> If there is a more elegant way to do this, please point me in the right
> >>> direction.  
> >>
> >> You might be aware of https://virtio-mem.gitlab.io/developer-guide.html 
> >> and the "issue" with 64k base pages - 512MB granularity. Similar as the 
> >> question from Auger, have you tried running arm64 with differing page 
> >> sizes in host/guest?
> >>
> > 
> > Hi David,
> > 
> >> With recent kernels, you can use "memhp_default_state=online_movable" on 
> >> the kernel cmdline to make memory unplug more likely to succeed - 
> >> especially with 64k base pages. You just have to be sure to not hotplug 
> >> "too much memory" to a VM.
> > 
> > Thanks for the pointer - that definitely simplifies testing.  Was getting a 
> > bit
> > tedious with out that.
> > 
> > As ever other stuff got in the way, so I only just got back to looking at 
> > this.
> > 
> > I've not done a particularly comprehensive set of tests yet, but things seem
> > to 'work' with mixed page sizes.
> > 
> > With 64K pages in general, you run into a problem with the device 
> > block_size being
> > smaller than the subblock_size.  I've just added a check for that into the
> 
> "device block size smaller than subblock size" - that's very common,
> e.g.,  on x86-64.
> 
> E.g., device_block_size is 2MiB, subblock size 4MiB - until we improve
> that in the future in Linux guests.
> 
> Or did you mean something else?
> 
> > virtio-mem kernel driver and have it fail to probe if that happens.  I don't
> > think such a setup makes any sense anyway so no loss there.  Should it make 
> > sense
> > to drop that restriction in the future we can deal with that then without 
> > breaking
> > backwards compatibility.
> > 
> > So the question is whether it makes sense to bother with virtio-mem support
> > at all on ARM64 with 64k pages given currently the minimum workable 
> > block_size
> > is 512MiB?  I guess there is an argument of virtio-mem being a possibly more
> > convenient interface than full memory HP.  Curious to hear what people 
> > think on
> > this?
> 
> IMHO we really want it. For example, RHEL is always 64k. This is a
> current guest limitation, to be improved in the future - either by
> moving away from 512MB huge pages with 64k or by improving
> alloc_contig_range().

Even with 64k pages you may be able to have 2MB huge pages by setting
default_hugepagesz=2M on the kernel command line.

Thanks,
drew

> 
> > 
> > 4K guest on 64K host seems fine and no such limit is needed - though there
> > may be performance issues doing that.
> 
> Yeah, if one is lucky to get one of these 512 MiB huge pages at all :)
> 
> > 
> > 64k guest on 4k host with 512MiB block size seems fine.
> > 
> > If there are any places anyone thinks need particular poking I'd appreciate 
> > a hint :)
> 
> If things seem to work for now, that's great :) Thanks!
> 
> -- 
> Thanks,
> 
> David / dhildenb
> 
> 




Re: [PATCH v2] hw/arm/virt enable support for virtio-mem

2020-11-24 Thread David Hildenbrand
On 24.11.20 19:11, Jonathan Cameron wrote:
> On Mon, 9 Nov 2020 20:47:09 +0100
> David Hildenbrand  wrote:
> 
> +CC Eric based on similar query in other branch of the thread.
> 
>> On 05.11.20 18:43, Jonathan Cameron wrote:
>>> Basically a cut and paste job from the x86 support with the exception of
>>> needing a larger block size as the Memory Block Size (MIN_SECTION_SIZE)
>>> on ARM64 in Linux is 1G.
>>>
>>> Tested:
>>> * In full emulation and with KVM on an arm64 server.
>>> * cold and hotplug for the virtio-mem-pci device.
>>> * Wide range of memory sizes, added at creation and later.
>>> * Fairly basic memory usage of memory added.  Seems to function as normal.
>>> * NUMA setup with virtio-mem-pci devices on each node.
>>> * Simple migration test.
>>>
>>> Related kernel patch just enables the Kconfig item for ARM64 as an
>>> alternative to x86 in drivers/virtio/Kconfig
>>>
>>> The original patches from David Hildenbrand stated that he thought it should
>>> work for ARM64 but it wasn't enabled in the kernel [1]
>>> It appears he was correct and everything 'just works'.
>>>
>>> The build system related stuff is intended to ensure virtio-mem support is
>>> not built for arm32 (build will fail due no defined block size).
>>> If there is a more elegant way to do this, please point me in the right
>>> direction.  
>>
>> You might be aware of https://virtio-mem.gitlab.io/developer-guide.html 
>> and the "issue" with 64k base pages - 512MB granularity. Similar as the 
>> question from Auger, have you tried running arm64 with differing page 
>> sizes in host/guest?
>>
> 
> Hi David,
> 
>> With recent kernels, you can use "memhp_default_state=online_movable" on 
>> the kernel cmdline to make memory unplug more likely to succeed - 
>> especially with 64k base pages. You just have to be sure to not hotplug 
>> "too much memory" to a VM.
> 
> Thanks for the pointer - that definitely simplifies testing.  Was getting a 
> bit
> tedious with out that.
> 
> As ever other stuff got in the way, so I only just got back to looking at 
> this.
> 
> I've not done a particularly comprehensive set of tests yet, but things seem
> to 'work' with mixed page sizes.
> 
> With 64K pages in general, you run into a problem with the device block_size 
> being
> smaller than the subblock_size.  I've just added a check for that into the

"device block size smaller than subblock size" - that's very common,
e.g.,  on x86-64.

E.g., device_block_size is 2MiB, subblock size 4MiB - until we improve
that in the future in Linux guests.

Or did you mean something else?

> virtio-mem kernel driver and have it fail to probe if that happens.  I don't
> think such a setup makes any sense anyway so no loss there.  Should it make 
> sense
> to drop that restriction in the future we can deal with that then without 
> breaking
> backwards compatibility.
> 
> So the question is whether it makes sense to bother with virtio-mem support
> at all on ARM64 with 64k pages given currently the minimum workable block_size
> is 512MiB?  I guess there is an argument of virtio-mem being a possibly more
> convenient interface than full memory HP.  Curious to hear what people think 
> on
> this?

IMHO we really want it. For example, RHEL is always 64k. This is a
current guest limitation, to be improved in the future - either by
moving away from 512MB huge pages with 64k or by improving
alloc_contig_range().

> 
> 4K guest on 64K host seems fine and no such limit is needed - though there
> may be performance issues doing that.

Yeah, if one is lucky to get one of these 512 MiB huge pages at all :)

> 
> 64k guest on 4k host with 512MiB block size seems fine.
> 
> If there are any places anyone thinks need particular poking I'd appreciate a 
> hint :)

If things seem to work for now, that's great :) Thanks!

-- 
Thanks,

David / dhildenb




Re: [PATCH v2] hw/arm/virt enable support for virtio-mem

2020-11-24 Thread Jonathan Cameron
On Mon, 9 Nov 2020 20:47:09 +0100
David Hildenbrand  wrote:

+CC Eric based on similar query in other branch of the thread.

> On 05.11.20 18:43, Jonathan Cameron wrote:
> > Basically a cut and paste job from the x86 support with the exception of
> > needing a larger block size as the Memory Block Size (MIN_SECTION_SIZE)
> > on ARM64 in Linux is 1G.
> > 
> > Tested:
> > * In full emulation and with KVM on an arm64 server.
> > * cold and hotplug for the virtio-mem-pci device.
> > * Wide range of memory sizes, added at creation and later.
> > * Fairly basic memory usage of memory added.  Seems to function as normal.
> > * NUMA setup with virtio-mem-pci devices on each node.
> > * Simple migration test.
> > 
> > Related kernel patch just enables the Kconfig item for ARM64 as an
> > alternative to x86 in drivers/virtio/Kconfig
> > 
> > The original patches from David Hildenbrand stated that he thought it should
> > work for ARM64 but it wasn't enabled in the kernel [1]
> > It appears he was correct and everything 'just works'.
> > 
> > The build system related stuff is intended to ensure virtio-mem support is
> > not built for arm32 (build will fail due no defined block size).
> > If there is a more elegant way to do this, please point me in the right
> > direction.  
> 
> You might be aware of https://virtio-mem.gitlab.io/developer-guide.html 
> and the "issue" with 64k base pages - 512MB granularity. Similar as the 
> question from Auger, have you tried running arm64 with differing page 
> sizes in host/guest?
> 

Hi David,

> With recent kernels, you can use "memhp_default_state=online_movable" on 
> the kernel cmdline to make memory unplug more likely to succeed - 
> especially with 64k base pages. You just have to be sure to not hotplug 
> "too much memory" to a VM.

Thanks for the pointer - that definitely simplifies testing.  Was getting a bit
tedious with out that.

As ever other stuff got in the way, so I only just got back to looking at this.

I've not done a particularly comprehensive set of tests yet, but things seem
to 'work' with mixed page sizes.

With 64K pages in general, you run into a problem with the device block_size 
being
smaller than the subblock_size.  I've just added a check for that into the
virtio-mem kernel driver and have it fail to probe if that happens.  I don't
think such a setup makes any sense anyway so no loss there.  Should it make 
sense
to drop that restriction in the future we can deal with that then without 
breaking
backwards compatibility.

So the question is whether it makes sense to bother with virtio-mem support
at all on ARM64 with 64k pages given currently the minimum workable block_size
is 512MiB?  I guess there is an argument of virtio-mem being a possibly more
convenient interface than full memory HP.  Curious to hear what people think on
this?

4K guest on 64K host seems fine and no such limit is needed - though there
may be performance issues doing that.

64k guest on 4k host with 512MiB block size seems fine.

If there are any places anyone thinks need particular poking I'd appreciate a 
hint :)

Jonathan



> 
> 
> I had my prototype living at
> 
> g...@github.com:davidhildenbrand/qemu.git / virtio-mem-arm64
> 
> which looks very similar to your patch. That is good :)
> 
> [...]
> 
> >   static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
> >   DeviceState *dev, Error **errp)
> >   {
> > @@ -2336,6 +2389,9 @@ static void 
> > virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
> >   if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> >   virt_memory_plug(hotplug_dev, dev, errp);
> >   }
> > +if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
> > +virt_virtio_md_pci_plug(hotplug_dev, dev, errp);
> > +}  
> 
> These better all be "else if".
> 
> >   if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
> >   PCIDevice *pdev = PCI_DEVICE(dev);
> >   
> > @@ -2363,6 +2419,11 @@ static void virt_dimm_unplug_request(HotplugHandler 
> > *hotplug_dev,
> >   goto out;
> >   }
> >   
> > +if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
> > +error_setg(_err,
> > +   "virtio-mem based memory devices cannot be unplugged.");
> > +goto out;
> > +}  
> 
> This should go into virt_machine_device_unplug_request_cb() instead, no?
> [...]
> 
> 




Re: [PATCH v2] hw/arm/virt enable support for virtio-mem

2020-11-09 Thread David Hildenbrand

On 05.11.20 18:43, Jonathan Cameron wrote:

Basically a cut and paste job from the x86 support with the exception of
needing a larger block size as the Memory Block Size (MIN_SECTION_SIZE)
on ARM64 in Linux is 1G.

Tested:
* In full emulation and with KVM on an arm64 server.
* cold and hotplug for the virtio-mem-pci device.
* Wide range of memory sizes, added at creation and later.
* Fairly basic memory usage of memory added.  Seems to function as normal.
* NUMA setup with virtio-mem-pci devices on each node.
* Simple migration test.

Related kernel patch just enables the Kconfig item for ARM64 as an
alternative to x86 in drivers/virtio/Kconfig

The original patches from David Hildenbrand stated that he thought it should
work for ARM64 but it wasn't enabled in the kernel [1]
It appears he was correct and everything 'just works'.

The build system related stuff is intended to ensure virtio-mem support is
not built for arm32 (build will fail due no defined block size).
If there is a more elegant way to do this, please point me in the right
direction.


You might be aware of https://virtio-mem.gitlab.io/developer-guide.html 
and the "issue" with 64k base pages - 512MB granularity. Similar as the 
question from Auger, have you tried running arm64 with differing page 
sizes in host/guest?


With recent kernels, you can use "memhp_default_state=online_movable" on 
the kernel cmdline to make memory unplug more likely to succeed - 
especially with 64k base pages. You just have to be sure to not hotplug 
"too much memory" to a VM.



I had my prototype living at

g...@github.com:davidhildenbrand/qemu.git / virtio-mem-arm64

which looks very similar to your patch. That is good :)

[...]


  static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
  DeviceState *dev, Error **errp)
  {
@@ -2336,6 +2389,9 @@ static void virt_machine_device_plug_cb(HotplugHandler 
*hotplug_dev,
  if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
  virt_memory_plug(hotplug_dev, dev, errp);
  }
+if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
+virt_virtio_md_pci_plug(hotplug_dev, dev, errp);
+}


These better all be "else if".


  if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
  PCIDevice *pdev = PCI_DEVICE(dev);
  
@@ -2363,6 +2419,11 @@ static void virt_dimm_unplug_request(HotplugHandler *hotplug_dev,

  goto out;
  }
  
+if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {

+error_setg(_err,
+   "virtio-mem based memory devices cannot be unplugged.");
+goto out;
+}


This should go into virt_machine_device_unplug_request_cb() instead, no?
[...]


--
Thanks,

David / dhildenb




Re: [PATCH v2] hw/arm/virt enable support for virtio-mem

2020-11-09 Thread Auger Eric
Hi Jonathan,

On 11/5/20 6:43 PM, Jonathan Cameron wrote:
> Basically a cut and paste job from the x86 support with the exception of
> needing a larger block size as the Memory Block Size (MIN_SECTION_SIZE)
> on ARM64 in Linux is 1G.
> 
> Tested:
> * In full emulation and with KVM on an arm64 server.
> * cold and hotplug for the virtio-mem-pci device.
> * Wide range of memory sizes, added at creation and later.
> * Fairly basic memory usage of memory added.  Seems to function as normal.
> * NUMA setup with virtio-mem-pci devices on each node.
> * Simple migration test.

I would add in the commit message that the hot-unplug of the device is
not supported.
> 
> Related kernel patch just enables the Kconfig item for ARM64 as an
> alternative to x86 in drivers/virtio/Kconfig
> 
> The original patches from David Hildenbrand stated that he thought it should
> work for ARM64 but it wasn't enabled in the kernel [1]
> It appears he was correct and everything 'just works'.
Did you try with 64kB page guest as well?
> 
> The build system related stuff is intended to ensure virtio-mem support is
> not built for arm32 (build will fail due no defined block size).
> If there is a more elegant way to do this, please point me in the right
> direction.
I guess you meant CONFIG_ARCH_VIRTIO_MEM_SUPPORTED introduction
> 
> [1] https://lore.kernel.org/linux-mm/20191212171137.13872-1-da...@redhat.com/
> 
> Signed-off-by: Jonathan Cameron 
> ---
>  default-configs/devices/aarch64-softmmu.mak |  1 +
>  hw/arm/Kconfig  |  1 +
>  hw/arm/virt.c   | 64 -
>  hw/virtio/Kconfig   |  4 ++
>  hw/virtio/virtio-mem.c  |  2 +
>  5 files changed, 71 insertions(+), 1 deletion(-)
> 
> diff --git a/default-configs/devices/aarch64-softmmu.mak 
> b/default-configs/devices/aarch64-softmmu.mak
> index 958b1e08e4..31d6128a29 100644
> --- a/default-configs/devices/aarch64-softmmu.mak
> +++ b/default-configs/devices/aarch64-softmmu.mak
> @@ -6,3 +6,4 @@ include arm-softmmu.mak
>  CONFIG_XLNX_ZYNQMP_ARM=y
>  CONFIG_XLNX_VERSAL=y
>  CONFIG_SBSA_REF=y
> +CONFIG_ARCH_VIRTIO_MEM_SUPPORTED=y
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index fdf4464b94..eeae77eee9 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -20,6 +20,7 @@ config ARM_VIRT
>  select PLATFORM_BUS
>  select SMBIOS
>  select VIRTIO_MMIO
> +select VIRTIO_MEM_SUPPORTED if ARCH_VIRTIO_MEM_SUPPORTED
>  select ACPI_PCI
>  select MEM_DEVICE
>  select DIMM
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 8abb385d4e..6c96d71106 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -73,9 +73,11 @@
>  #include "hw/acpi/acpi.h"
>  #include "target/arm/internals.h"
>  #include "hw/mem/pc-dimm.h"
> +#include "hw/mem/memory-device.h"
>  #include "hw/mem/nvdimm.h"
>  #include "hw/acpi/generic_event_device.h"
>  #include "hw/virtio/virtio-iommu.h"
> +#include "hw/virtio/virtio-mem-pci.h"
>  #include "hw/char/pl011.h"
>  #include "qemu/guest-random.h"
>  
> @@ -2286,6 +2288,34 @@ static void virt_memory_plug(HotplugHandler 
> *hotplug_dev,
>   dev, _abort);
>  }
>  
> +static void virt_virtio_md_pci_pre_plug(HotplugHandler *hotplug_dev,
> +  DeviceState *dev, Error **errp)
> +{
> +HotplugHandler *hotplug_dev2 = qdev_get_bus_hotplug_handler(dev);
> +Error *local_err = NULL;
> +
> +if (!hotplug_dev2 && dev->hotplugged) {
> +/*
> + * Without a bus hotplug handler, we cannot control the plug/unplug
> + * order. We should never reach this point when hotplugging,
> + * however, better add a safety net.
> + */
> +error_setg(errp, "hotplug of virtio-mem based memory devices not"
> +   " supported on this bus.");
> +return;
> +}
> +/*
> + * First, see if we can plug this memory device at all. If that
> + * succeeds, branch of to the actual hotplug handler.
> + */
> +memory_device_pre_plug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev), NULL,
> +   _err);
> +if (!local_err && hotplug_dev2) {
> +hotplug_handler_pre_plug(hotplug_dev2, dev, _err);
> +}
> +error_propagate(errp, local_err);
> +}
> +
>  static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>  DeviceState *dev, Error **errp)
>  {
> @@ -2293,6 +2323,8 @@ static void 
> virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>  
>  if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>  virt_memory_pre_plug(hotplug_dev, dev, errp);
> +} else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
> +virt_virtio_md_pci_pre_plug(hotplug_dev, dev, errp);
>  } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
>  hwaddr db_start = 0, db_end = 0;
>  char 

Re: [PATCH v2] hw/arm/virt enable support for virtio-mem

2020-11-05 Thread Philippe Mathieu-Daudé
Cc'ing Igor.

On 11/5/20 6:43 PM, Jonathan Cameron wrote:
> Basically a cut and paste job from the x86 support with the exception of
> needing a larger block size as the Memory Block Size (MIN_SECTION_SIZE)
> on ARM64 in Linux is 1G.
> 
> Tested:
> * In full emulation and with KVM on an arm64 server.
> * cold and hotplug for the virtio-mem-pci device.
> * Wide range of memory sizes, added at creation and later.
> * Fairly basic memory usage of memory added.  Seems to function as normal.
> * NUMA setup with virtio-mem-pci devices on each node.
> * Simple migration test.
> 
> Related kernel patch just enables the Kconfig item for ARM64 as an
> alternative to x86 in drivers/virtio/Kconfig
> 
> The original patches from David Hildenbrand stated that he thought it should
> work for ARM64 but it wasn't enabled in the kernel [1]
> It appears he was correct and everything 'just works'.
> 
> The build system related stuff is intended to ensure virtio-mem support is
> not built for arm32 (build will fail due no defined block size).
> If there is a more elegant way to do this, please point me in the right
> direction.
> 
> [1] https://lore.kernel.org/linux-mm/20191212171137.13872-1-da...@redhat.com/
> 
> Signed-off-by: Jonathan Cameron 
> ---
>  default-configs/devices/aarch64-softmmu.mak |  1 +
>  hw/arm/Kconfig  |  1 +
>  hw/arm/virt.c   | 64 -
>  hw/virtio/Kconfig   |  4 ++
>  hw/virtio/virtio-mem.c  |  2 +
>  5 files changed, 71 insertions(+), 1 deletion(-)
> 
> diff --git a/default-configs/devices/aarch64-softmmu.mak 
> b/default-configs/devices/aarch64-softmmu.mak
> index 958b1e08e4..31d6128a29 100644
> --- a/default-configs/devices/aarch64-softmmu.mak
> +++ b/default-configs/devices/aarch64-softmmu.mak
> @@ -6,3 +6,4 @@ include arm-softmmu.mak
>  CONFIG_XLNX_ZYNQMP_ARM=y
>  CONFIG_XLNX_VERSAL=y
>  CONFIG_SBSA_REF=y
> +CONFIG_ARCH_VIRTIO_MEM_SUPPORTED=y
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index fdf4464b94..eeae77eee9 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -20,6 +20,7 @@ config ARM_VIRT
>  select PLATFORM_BUS
>  select SMBIOS
>  select VIRTIO_MMIO
> +select VIRTIO_MEM_SUPPORTED if ARCH_VIRTIO_MEM_SUPPORTED
>  select ACPI_PCI
>  select MEM_DEVICE
>  select DIMM
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 8abb385d4e..6c96d71106 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -73,9 +73,11 @@
>  #include "hw/acpi/acpi.h"
>  #include "target/arm/internals.h"
>  #include "hw/mem/pc-dimm.h"
> +#include "hw/mem/memory-device.h"
>  #include "hw/mem/nvdimm.h"
>  #include "hw/acpi/generic_event_device.h"
>  #include "hw/virtio/virtio-iommu.h"
> +#include "hw/virtio/virtio-mem-pci.h"
>  #include "hw/char/pl011.h"
>  #include "qemu/guest-random.h"
>  
> @@ -2286,6 +2288,34 @@ static void virt_memory_plug(HotplugHandler 
> *hotplug_dev,
>   dev, _abort);
>  }
>  
> +static void virt_virtio_md_pci_pre_plug(HotplugHandler *hotplug_dev,
> +  DeviceState *dev, Error **errp)
> +{
> +HotplugHandler *hotplug_dev2 = qdev_get_bus_hotplug_handler(dev);
> +Error *local_err = NULL;
> +
> +if (!hotplug_dev2 && dev->hotplugged) {
> +/*
> + * Without a bus hotplug handler, we cannot control the plug/unplug
> + * order. We should never reach this point when hotplugging,
> + * however, better add a safety net.
> + */
> +error_setg(errp, "hotplug of virtio-mem based memory devices not"
> +   " supported on this bus.");
> +return;
> +}
> +/*
> + * First, see if we can plug this memory device at all. If that
> + * succeeds, branch of to the actual hotplug handler.
> + */
> +memory_device_pre_plug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev), NULL,
> +   _err);
> +if (!local_err && hotplug_dev2) {
> +hotplug_handler_pre_plug(hotplug_dev2, dev, _err);
> +}
> +error_propagate(errp, local_err);
> +}
> +
>  static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>  DeviceState *dev, Error **errp)
>  {
> @@ -2293,6 +2323,8 @@ static void 
> virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>  
>  if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>  virt_memory_pre_plug(hotplug_dev, dev, errp);
> +} else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
> +virt_virtio_md_pci_pre_plug(hotplug_dev, dev, errp);
>  } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
>  hwaddr db_start = 0, db_end = 0;
>  char *resv_prop_str;
> @@ -2322,6 +2354,27 @@ static void 
> virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>  }
>  }
>  
> +static void virt_virtio_md_pci_plug(HotplugHandler *hotplug_dev,
> + 

[PATCH v2] hw/arm/virt enable support for virtio-mem

2020-11-05 Thread Jonathan Cameron
Basically a cut and paste job from the x86 support with the exception of
needing a larger block size as the Memory Block Size (MIN_SECTION_SIZE)
on ARM64 in Linux is 1G.

Tested:
* In full emulation and with KVM on an arm64 server.
* cold and hotplug for the virtio-mem-pci device.
* Wide range of memory sizes, added at creation and later.
* Fairly basic memory usage of memory added.  Seems to function as normal.
* NUMA setup with virtio-mem-pci devices on each node.
* Simple migration test.

Related kernel patch just enables the Kconfig item for ARM64 as an
alternative to x86 in drivers/virtio/Kconfig

The original patches from David Hildenbrand stated that he thought it should
work for ARM64 but it wasn't enabled in the kernel [1]
It appears he was correct and everything 'just works'.

The build system related stuff is intended to ensure virtio-mem support is
not built for arm32 (build will fail due no defined block size).
If there is a more elegant way to do this, please point me in the right
direction.

[1] https://lore.kernel.org/linux-mm/20191212171137.13872-1-da...@redhat.com/

Signed-off-by: Jonathan Cameron 
---
 default-configs/devices/aarch64-softmmu.mak |  1 +
 hw/arm/Kconfig  |  1 +
 hw/arm/virt.c   | 64 -
 hw/virtio/Kconfig   |  4 ++
 hw/virtio/virtio-mem.c  |  2 +
 5 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/default-configs/devices/aarch64-softmmu.mak 
b/default-configs/devices/aarch64-softmmu.mak
index 958b1e08e4..31d6128a29 100644
--- a/default-configs/devices/aarch64-softmmu.mak
+++ b/default-configs/devices/aarch64-softmmu.mak
@@ -6,3 +6,4 @@ include arm-softmmu.mak
 CONFIG_XLNX_ZYNQMP_ARM=y
 CONFIG_XLNX_VERSAL=y
 CONFIG_SBSA_REF=y
+CONFIG_ARCH_VIRTIO_MEM_SUPPORTED=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index fdf4464b94..eeae77eee9 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -20,6 +20,7 @@ config ARM_VIRT
 select PLATFORM_BUS
 select SMBIOS
 select VIRTIO_MMIO
+select VIRTIO_MEM_SUPPORTED if ARCH_VIRTIO_MEM_SUPPORTED
 select ACPI_PCI
 select MEM_DEVICE
 select DIMM
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 8abb385d4e..6c96d71106 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -73,9 +73,11 @@
 #include "hw/acpi/acpi.h"
 #include "target/arm/internals.h"
 #include "hw/mem/pc-dimm.h"
+#include "hw/mem/memory-device.h"
 #include "hw/mem/nvdimm.h"
 #include "hw/acpi/generic_event_device.h"
 #include "hw/virtio/virtio-iommu.h"
+#include "hw/virtio/virtio-mem-pci.h"
 #include "hw/char/pl011.h"
 #include "qemu/guest-random.h"
 
@@ -2286,6 +2288,34 @@ static void virt_memory_plug(HotplugHandler *hotplug_dev,
  dev, _abort);
 }
 
+static void virt_virtio_md_pci_pre_plug(HotplugHandler *hotplug_dev,
+  DeviceState *dev, Error **errp)
+{
+HotplugHandler *hotplug_dev2 = qdev_get_bus_hotplug_handler(dev);
+Error *local_err = NULL;
+
+if (!hotplug_dev2 && dev->hotplugged) {
+/*
+ * Without a bus hotplug handler, we cannot control the plug/unplug
+ * order. We should never reach this point when hotplugging,
+ * however, better add a safety net.
+ */
+error_setg(errp, "hotplug of virtio-mem based memory devices not"
+   " supported on this bus.");
+return;
+}
+/*
+ * First, see if we can plug this memory device at all. If that
+ * succeeds, branch of to the actual hotplug handler.
+ */
+memory_device_pre_plug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev), NULL,
+   _err);
+if (!local_err && hotplug_dev2) {
+hotplug_handler_pre_plug(hotplug_dev2, dev, _err);
+}
+error_propagate(errp, local_err);
+}
+
 static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
 DeviceState *dev, Error **errp)
 {
@@ -2293,6 +2323,8 @@ static void 
virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
 
 if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
 virt_memory_pre_plug(hotplug_dev, dev, errp);
+} else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
+virt_virtio_md_pci_pre_plug(hotplug_dev, dev, errp);
 } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
 hwaddr db_start = 0, db_end = 0;
 char *resv_prop_str;
@@ -2322,6 +2354,27 @@ static void 
virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
 }
 }
 
+static void virt_virtio_md_pci_plug(HotplugHandler *hotplug_dev,
+  DeviceState *dev, Error **errp)
+{
+HotplugHandler *hotplug_dev2 = qdev_get_bus_hotplug_handler(dev);
+Error *local_err = NULL;
+
+/*
+ * Plug the memory device first and then branch off to the actual
+ * hotplug handler. If that one fails, we can easily