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
