| Message ID | 20231026034042.812006-1-shahuang@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvm-unit-tests,v1] configure: arm64: Add support for dirty-ring in migration | expand |
On 26/10/2023 05.40, Shaoqin Huang wrote: > Add a new configure option "--dirty-ring-size" to support dirty-ring > migration on arm64. By default, the dirty-ring is disabled, we can > enable it by: > > # ./configure --dirty-ring-size=65536 > > This will generate one more entry in config.mak, it will look like: > > # cat config.mak > : > ACCEL=kvm,dirty-ring-size=65536 > > With this configure option, user can easy enable dirty-ring and specify > dirty-ring-size to test the dirty-ring in migration. Do we really need a separate configure switch for this? If it is just about setting a value in the ACCEL variable, you can also run the tests like this: ACCEL=kvm,dirty-ring-size=65536 ./run_tests.sh Thomas
On 10/26/23 13:12, Thomas Huth wrote: > On 26/10/2023 05.40, Shaoqin Huang wrote: >> Add a new configure option "--dirty-ring-size" to support dirty-ring >> migration on arm64. By default, the dirty-ring is disabled, we can >> enable it by: >> >> # ./configure --dirty-ring-size=65536 >> >> This will generate one more entry in config.mak, it will look like: >> >> # cat config.mak >> : >> ACCEL=kvm,dirty-ring-size=65536 >> >> With this configure option, user can easy enable dirty-ring and specify >> dirty-ring-size to test the dirty-ring in migration. > > Do we really need a separate configure switch for this? If it is just > about setting a value in the ACCEL variable, you can also run the tests > like this: > > ACCEL=kvm,dirty-ring-size=65536 ./run_tests.sh > > Thomas > Hi Thomas, You're right. We can do it by simply set the ACCEL when execute ./run_tests.sh. I think maybe add a configure can make auto test to set the dirty-ring easier? but I'm not 100% sure it will benefit to them. Thanks, Shaoqin
On Thu, Oct 26, 2023 at 01:54:55PM +0800, Shaoqin Huang wrote: > > > On 10/26/23 13:12, Thomas Huth wrote: > > On 26/10/2023 05.40, Shaoqin Huang wrote: > > > Add a new configure option "--dirty-ring-size" to support dirty-ring > > > migration on arm64. By default, the dirty-ring is disabled, we can > > > enable it by: > > > > > > # ./configure --dirty-ring-size=65536 > > > > > > This will generate one more entry in config.mak, it will look like: > > > > > > # cat config.mak > > > : > > > ACCEL=kvm,dirty-ring-size=65536 > > > > > > With this configure option, user can easy enable dirty-ring and specify > > > dirty-ring-size to test the dirty-ring in migration. > > > > Do we really need a separate configure switch for this? If it is just > > about setting a value in the ACCEL variable, you can also run the tests > > like this: > > > > ACCEL=kvm,dirty-ring-size=65536 ./run_tests.sh > > > > Thomas > > > > Hi Thomas, > > You're right. We can do it by simply set the ACCEL when execute > ./run_tests.sh. I think maybe add a configure can make auto test to set the > dirty-ring easier? but I'm not 100% sure it will benefit to them. > For unit tests that require specific configurations, those configurations should be added to the unittests.cfg file. As we don't currently support adding accel properties, we should add a new parameter and extend the parsing. Thanks, drew
Hi drew, On 10/26/23 15:40, Andrew Jones wrote: > On Thu, Oct 26, 2023 at 01:54:55PM +0800, Shaoqin Huang wrote: >> >> >> On 10/26/23 13:12, Thomas Huth wrote: >>> On 26/10/2023 05.40, Shaoqin Huang wrote: >>>> Add a new configure option "--dirty-ring-size" to support dirty-ring >>>> migration on arm64. By default, the dirty-ring is disabled, we can >>>> enable it by: >>>> >>>> # ./configure --dirty-ring-size=65536 >>>> >>>> This will generate one more entry in config.mak, it will look like: >>>> >>>> # cat config.mak >>>> : >>>> ACCEL=kvm,dirty-ring-size=65536 >>>> >>>> With this configure option, user can easy enable dirty-ring and specify >>>> dirty-ring-size to test the dirty-ring in migration. >>> >>> Do we really need a separate configure switch for this? If it is just >>> about setting a value in the ACCEL variable, you can also run the tests >>> like this: >>> >>> ACCEL=kvm,dirty-ring-size=65536 ./run_tests.sh >>> >>> Thomas >>> >> >> Hi Thomas, >> >> You're right. We can do it by simply set the ACCEL when execute >> ./run_tests.sh. I think maybe add a configure can make auto test to set the >> dirty-ring easier? but I'm not 100% sure it will benefit to them. >> > > For unit tests that require specific configurations, those configurations > should be added to the unittests.cfg file. As we don't currently support > adding accel properties, we should add a new parameter and extend the > parsing. So you mean we add the accel properties into the unittests.cfg like: accel = kvm,dirty-ring-size=65536 Then let the `for_each_unittest` to parse the parameter? In this way, should we copy the migration config to dirty-ring migration? Just like: [its-migration] file = gic.flat smp = $MAX_SMP extra_params = -machine gic-version=3 -append 'its-migration' groups = its migration arch = arm64 [its-migration-dirty-ring] file = gic.flat smp = $MAX_SMP extra_params = -machine gic-version=3 -append 'its-migration' groups = its migration arch = arm64 accel = kvm,dirty-ring-size=65536 So it will test both dirty bitmap and dirty ring. Thanks, Shaoqin > > Thanks, > drew >
On Thu, Oct 26, 2023 at 04:44:26PM +0800, Shaoqin Huang wrote: > Hi drew, > > On 10/26/23 15:40, Andrew Jones wrote: > > On Thu, Oct 26, 2023 at 01:54:55PM +0800, Shaoqin Huang wrote: > > > > > > > > > On 10/26/23 13:12, Thomas Huth wrote: > > > > On 26/10/2023 05.40, Shaoqin Huang wrote: > > > > > Add a new configure option "--dirty-ring-size" to support dirty-ring > > > > > migration on arm64. By default, the dirty-ring is disabled, we can > > > > > enable it by: > > > > > > > > > > # ./configure --dirty-ring-size=65536 > > > > > > > > > > This will generate one more entry in config.mak, it will look like: > > > > > > > > > > # cat config.mak > > > > > : > > > > > ACCEL=kvm,dirty-ring-size=65536 > > > > > > > > > > With this configure option, user can easy enable dirty-ring and specify > > > > > dirty-ring-size to test the dirty-ring in migration. > > > > > > > > Do we really need a separate configure switch for this? If it is just > > > > about setting a value in the ACCEL variable, you can also run the tests > > > > like this: > > > > > > > > ACCEL=kvm,dirty-ring-size=65536 ./run_tests.sh > > > > > > > > Thomas > > > > > > > > > > Hi Thomas, > > > > > > You're right. We can do it by simply set the ACCEL when execute > > > ./run_tests.sh. I think maybe add a configure can make auto test to set the > > > dirty-ring easier? but I'm not 100% sure it will benefit to them. > > > > > > > For unit tests that require specific configurations, those configurations > > should be added to the unittests.cfg file. As we don't currently support > > adding accel properties, we should add a new parameter and extend the > > parsing. > > So you mean we add the accel properties into the unittests.cfg like: > > accel = kvm,dirty-ring-size=65536 > > Then let the `for_each_unittest` to parse the parameter? > In this way, should we copy the migration config to dirty-ring migration? > Just like: > > [its-migration] > file = gic.flat > smp = $MAX_SMP > extra_params = -machine gic-version=3 -append 'its-migration' > groups = its migration > arch = arm64 > > [its-migration-dirty-ring] > file = gic.flat > smp = $MAX_SMP > extra_params = -machine gic-version=3 -append 'its-migration' > groups = its migration > arch = arm64 > accel = kvm,dirty-ring-size=65536 > > So it will test both dirty bitmap and dirty ring. Yup, either like you've outlined above with modifying the definition of 'accel' to take an optional [,params...] or by creating another parameter, e.g. 'accel_props' and then doing accel = kvm accel_props = dirty-ring-size=65536 Which of the two approaches to choose depends on how intrusive the modification of 'accel' would be and how well it would preserve its current behavior. Thanks, drew
diff --git a/configure b/configure index 6ee9b27..54ce38a 100755 --- a/configure +++ b/configure @@ -32,6 +32,7 @@ enable_dump=no page_size= earlycon= efi= +dirty_ring_size= # Enable -Werror by default for git repositories only (i.e. developer builds) if [ -e "$srcdir"/.git ]; then @@ -89,6 +90,9 @@ usage() { --[enable|disable]-efi Boot and run from UEFI (disabled by default, x86_64 and arm64 only) --[enable|disable]-werror Select whether to compile with the -Werror compiler flag + --dirty-ring-size=SIZE + Specify the dirty-ring size and enable dirty-ring for + migration(disable dirty-ring by default, arm64 only) EOF exit 1 } @@ -174,6 +178,9 @@ while [[ "$1" = -* ]]; do --disable-werror) werror= ;; + --dirty-ring-size) + dirty_ring_size="$arg" + ;; --help) usage ;; @@ -213,6 +220,27 @@ if [ "$efi" ] && [ "$arch" != "x86_64" ] && [ "$arch" != "arm64" ]; then usage fi +if [ "$dirty_ring_size" ]; then + if [ "$arch" != "arm64" ]; then + echo "--dirty-ring-size is not supported for $arch" + usage + fi + _size=$dirty_ring_size + if [ ! "${_size//[0-9]}" ]; then + if (( _size < 1024 )); then + echo "--dirty-ring-size suggest minimum value is 1024" + usage + fi + if (( _size & (_size - 1) )); then + echo "--dirty-ring-size must be a power of two" + usage + fi + else + echo "--dirty-ring-size must be positive number and a power of two" + usage + fi +fi + if [ -z "$page_size" ]; then if [ "$efi" = 'y' ] && [ "$arch" = "arm64" ]; then page_size="4096" @@ -419,6 +447,9 @@ EOF if [ "$arch" = "arm" ] || [ "$arch" = "arm64" ]; then echo "TARGET=$target" >> config.mak fi +if [ "$arch" = "arm64" ] && [ "$dirty_ring_size" ]; then + echo "ACCEL=kvm,dirty-ring-size=$dirty_ring_size" >> config.mak +fi cat <<EOF > lib/config.h #ifndef _CONFIG_H_
Add a new configure option "--dirty-ring-size" to support dirty-ring migration on arm64. By default, the dirty-ring is disabled, we can enable it by: # ./configure --dirty-ring-size=65536 This will generate one more entry in config.mak, it will look like: # cat config.mak : ACCEL=kvm,dirty-ring-size=65536 With this configure option, user can easy enable dirty-ring and specify dirty-ring-size to test the dirty-ring in migration. Signed-off-by: Shaoqin Huang <shahuang@redhat.com> --- configure | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)