On 03/06/2017 10:43 AM, Alex Bennée wrote: > > Frederic Konrad <[email protected]> writes: > >> Hi All, >> >> I've a strangeness with the "drop iolock" patch. >> It seems it has an impact on the MMIO execution patch-set I'm working >> on: > > Hmm the only real change is the BQL (implicit or explict) being taken on > entry to the mmio functions.
Yes maybe it just trigger a bug more easily.. Fred > >> Basically modifying the memory tree is causing a Bad Ram Address error. >> I wonder if this can't happen with hotplug/unplug model as well. > > Isn't this all done under an RCU? I don't think any of this should have > changed for the MTTCG patch series. > >> I'll look into this. >> Shoot if you have any evidence of why that doesn't work :). > > --enable-tcg-debug will turn on more lock checking (at a performance > hit). You could try that. > >> >> Thanks, >> Fred >> >> On 03/02/2017 08:53 PM, Alex Bennée wrote: >>> Hi Peter, >>> >>> So perhaps predictably the merging of MTTCG has thrown up some issues >>> during soft-freeze. The following patches fix up some of those: >>> >>> The following where in v1 and just have r-b tags: >>> >>> vl/cpus: be smarter with icount and MTTCG >>> target/i386/cpu.h: declare TCG_GUEST_DEFAULT_MO >>> cpus.c: add additional error_report when !TARGET_SUPPORT_MTTCG >>> >>> The next change downgrades the asserts to tcg_debug_asserts. This will >>> reduce the instances of production builds failing on non-MTTCG enabled >>> guests. The races still need to be fixed but you now have to >>> --enable-debug-tcg to go hunting for them: >>> >>> translate: downgrade IRQ BQL asserts to tcg_debug_assert >>> >>> This never came up in my testing but it seems some guests can >>> translate while doing a tlb_fill. The call to cpu_restore_state never >>> worked before (as we are translating the block you are looking for) >>> but by bugging out we avoid the double tb_lock(). >>> >>> translate-all: exit cpu_restore_state early if translating >>> >>> Then we have a bunch of fixes from the reports on the list. They are >>> safe to merge but I'll leave that up to the maintainers. There may be >>> an argument for holding these patches back until the guest is >>> converted to be MTTCG aware and a full audit of the CPU<->HW emulation >>> boundaries is done for each one: >>> >>> sparc/sparc64: grab BQL before calling cpu_check_irqs >>> s390x/misc_helper.c: wrap IO instructions in BQL >>> target/xtensa: hold BQL for interrupt processing >>> target/mips/op_helper: hold BQL before calling cpu_mips_get_count >>> >>> Finally these are just tiny debug fixes while investigating the icount >>> regression: >>> >>> target/arm/helper: make it clear the EC field is also in hex >>> hw/intc/arm_gic: modernise the DPRINTF >>> >>> Going forward I think 1-5 are ready to be merged now. For 6-9 we >>> should await decisions from the target maintainers. The last two are >>> entirely up to you. >>> >>> It looks like the -icount regression is a poor interaction between >>> icount timers and the BQL but this is going to require discussion with >>> Paolo in the morning. >>> >>> Cheers, >>> >>> >>> Alex Bennée (11): >>> vl/cpus: be smarter with icount and MTTCG >>> target/i386/cpu.h: declare TCG_GUEST_DEFAULT_MO >>> cpus.c: add additional error_report when !TARGET_SUPPORT_MTTCG >>> translate: downgrade IRQ BQL asserts to tcg_debug_assert >>> translate-all: exit cpu_restore_state early if translating >>> sparc/sparc64: grab BQL before calling cpu_check_irqs >>> s390x/misc_helper.c: wrap IO instructions in BQL >>> target/xtensa: hold BQL for interrupt processing >>> target/mips/op_helper: hold BQL before calling cpu_mips_get_count >>> target/arm/helper: make it clear the EC field is also in hex >>> hw/intc/arm_gic: modernise the DPRINTF >>> >>> cpus.c | 11 +++++++---- >>> hw/intc/arm_gic.c | 13 +++++++++---- >>> hw/sparc/sun4m.c | 3 +++ >>> hw/sparc64/sparc64.c | 3 +++ >>> target/arm/helper.c | 2 +- >>> target/i386/cpu.h | 3 +++ >>> target/mips/op_helper.c | 17 ++++++++++++++--- >>> target/s390x/misc_helper.c | 21 +++++++++++++++++++++ >>> target/sparc/int64_helper.c | 3 +++ >>> target/sparc/win_helper.c | 11 +++++++++++ >>> target/xtensa/helper.c | 1 + >>> target/xtensa/op_helper.c | 7 +++++++ >>> translate-all.c | 9 ++++++++- >>> translate-common.c | 3 ++- >>> vl.c | 7 ++----- >>> 15 files changed, 95 insertions(+), 19 deletions(-) >>> > > > -- > Alex Bennée >
