From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60985) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bkYSR-0003er-3J for qemu-devel@nongnu.org; Thu, 15 Sep 2016 11:20:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bkYSL-0004Ex-6L for qemu-devel@nongnu.org; Thu, 15 Sep 2016 11:20:09 -0400 Received: from mail-lf0-f67.google.com ([209.85.215.67]:34146) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bkYSK-0004Eh-UB for qemu-devel@nongnu.org; Thu, 15 Sep 2016 11:20:05 -0400 Received: by mail-lf0-f67.google.com with SMTP id k12so3083665lfb.1 for ; Thu, 15 Sep 2016 08:20:04 -0700 (PDT) Sender: Paolo Bonzini References: <147377800565.11859.4411044563640180545.stgit@brijesh-build-machine> <147377816100.11859.1924921034992764815.stgit@brijesh-build-machine> <1911fbd8-4476-c733-2972-0210a0afff80@redhat.com> <98729cf1-34ab-f0dd-7961-5e5efa2380b0@amd.com> <362908f3-69dc-5b8f-5976-95aba035f7c6@redhat.com> <269e58f7-6df3-6f84-a737-b7f441b0fa52@amd.com> <90efced4-3a77-d28b-e1fe-5a937bcf991b@redhat.com> <44c5f5f1-4697-6adb-4f4f-7203398bdd3b@amd.com> From: Paolo Bonzini Message-ID: Date: Thu, 15 Sep 2016 17:19:01 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [RFC PATCH v1 15/22] i386: sev: register RAM read/write ops for BIOS and PC.RAM region List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Brijesh Singh , ehabkost@redhat.com, crosthwaite.peter@gmail.com, armbru@redhat.com, mst@redhat.com, p.fedin@samsung.com, qemu-devel@nongnu.org, lcapitulino@redhat.com, rth@twiddle.net On 15/09/2016 16:13, Brijesh Singh wrote: > 1) something like this > > diff --git a/target-i386/helper.c b/target-i386/helper.c > index a9d8aef..6322265 100644 > --- a/target-i386/helper.c > +++ b/target-i386/helper.c > @@ -1379,13 +1379,22 @@ void x86_cpu_exec_exit(CPUState *cs) > } > > #ifndef CONFIG_USER_ONLY > +static inline MemTxAttrs get_mem_debug_attrs(CPUX86State *env) > +{ > + MemTxAttrs attrs = cpu_get_mem_attrs(env); > + > + attrs.debug = MEMTXATTRS_DEBUG; > + > + return attrs; > +} > + > uint8_t x86_ldub_phys(CPUState *cs, hwaddr addr) > { > X86CPU *cpu = X86_CPU(cs); > CPUX86State *env = &cpu->env; > > return address_space_ldub(cs->as, addr, > - cpu_get_mem_attrs(env), > + get_mem_debug_attrs(env), > NULL); > } This changes the semantics of x86_ld*_phys so it's not acceptable. You need new exec.c functions that wrap cpu_physical_memory_rw_debug_internal so that they end up calling your hooks. Like this: x86_cpu_get_phys_page_debug -> calls ldl_phys_debug -> calls cpu_physical_memory_rw_debug_internal uint32_t ldl_phys_debug(CPUState *cs, hwaddr addr) { /* This really should be a little more complex to support * SMRAM, but this is enough for now. I'll provide you with a * diff when you post v2. */ MemTxAttrs attrs = { .debug = 1 }; int asidx = cpu_asidx_from_attrs(cpu, attrs); uint32_t val; cpu_physical_memory_rw_debug_internal(cpu->cpu_ases[asidx].as, addr, (void *) &val, 4, READ_DATA); return tswap32(val); } /* and the same for ldq_phys_debug */ These two new functions can replace x86_ld*_phys in x86_cpu_get_phys_page_debug. Technically I'm not even sure you need the new .debug attribute now that the hooks are never called by the device emulation DMA path. Still, asserting something like assert(attrs->debug || sev_info.state == SEV_LAUNCH_START) in your hooks is probably a good idea. Even if you only need the new attribute for that assertion, keep it. It's just a couple lines of code. Paolo