From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 95D12C433F5 for ; Tue, 26 Oct 2021 07:50:41 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 3525B60F22 for ; Tue, 26 Oct 2021 07:50:41 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 3525B60F22 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=csgraf.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=nongnu.org Received: from localhost ([::1]:59910 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mfHEG-0006vB-9u for qemu-devel@archiver.kernel.org; Tue, 26 Oct 2021 03:50:40 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:56956) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mfGae-0001lK-Af for qemu-devel@nongnu.org; Tue, 26 Oct 2021 03:09:44 -0400 Received: from mail.csgraf.de ([85.25.223.15]:49088 helo=zulu616.server4you.de) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mfGaZ-0007Jy-NX for qemu-devel@nongnu.org; Tue, 26 Oct 2021 03:09:43 -0400 Received: from [192.168.106.118] (dynamic-077-007-071-240.77.7.pool.telefonica.de [77.7.71.240]) by csgraf.de (Postfix) with ESMTPSA id E0D346080126; Tue, 26 Oct 2021 09:09:35 +0200 (CEST) Message-ID: <1d84fe5e-1933-8798-ff42-e752ea4e5943@csgraf.de> Date: Tue, 26 Oct 2021 09:09:34 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.2.1 Subject: Re: [PATCH] hvf: arm: Ignore cache operations on MMIO Content-Language: en-US To: Richard Henderson , Cameron Esfahani References: <20211025191349.52992-1-agraf@csgraf.de> From: Alexander Graf In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Received-SPF: pass client-ip=85.25.223.15; envelope-from=agraf@csgraf.de; helo=zulu616.server4you.de X-Spam_score_int: -46 X-Spam_score: -4.7 X-Spam_bar: ---- X-Spam_report: (-4.7 / 5.0 requ) BAYES_00=-1.9, NICE_REPLY_A=-2.846, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Paolo Bonzini , kettenis@openbsd.org, Roman Bolshakov , qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" On 26.10.21 02:14, Richard Henderson wrote: > On 10/25/21 12:13 PM, Alexander Graf wrote: >> +    /* >> +     * We ran into an instruction that traps for data, but is not >> +     * hardware predecoded. This should not ever happen for well >> +     * behaved guests. Let's try to see if we can somehow rescue >> +     * the situation. >> +     */ >> + >> +    cpu_synchronize_state(cpu); >> +    if (cpu_memory_rw_debug(cpu, env->pc, &insn, 4, 0)) { > > This isn't correct, since this would be a physical address access, and > env->pc is virtual. Yes, hence cpu_memory_rw_debug which accesses virtual memory: https://git.qemu.org/?p=qemu.git;a=blob;f=softmmu/physmem.c#l3418 > > Phil's idea of cpu_ldl_data may be correct, and cpu_ldl_code may be > slightly more so, because we got EC_DATAABORT not EC_INSNABORT, which > means that the virtual address at env->pc is mapped and executable. > > However, in the event that there's some sort of race condition in > between this data abort and hvf stopping all threads for the vm exit, > by which the page tables could have been modified between here and > there, then cpu_ldl_code *could* produce another exception. > > In which case the interface that gdbstub uses, cc->memory_rw_debug, > will be most correct. I don't believe that one is implemented for arm, correct? > > >> @@ -1156,6 +1183,11 @@ int hvf_vcpu_exec(CPUState *cpu) >> hvf_exit->exception.physical_address, isv, >>                                iswrite, s1ptw, len, srt); >>   +        if (!isv) { >> +            g_assert(hvf_emulate_insn(cpu)); >> +            advance_pc = true; >> +            break; >> +        } >>           assert(isv); > > Ouch.  HVF really passes along an invalid syndrome?  I was expecting > that you'd be able to avoid all of the instruction parsing and check > syndrome.cm (bit 8) for a cache management instruction. That's a very subtle way of telling me I'm stupid :). Thanks for the catch! Using the CM bit is obviously way better. Let me build v2. Alex