From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57466) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bkIKF-0005zv-0r for qemu-devel@nongnu.org; Wed, 14 Sep 2016 18:06:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bkIK8-0001ae-Vq for qemu-devel@nongnu.org; Wed, 14 Sep 2016 18:06:37 -0400 Received: from mail-by2nam03on0047.outbound.protection.outlook.com ([104.47.42.47]:43568 helo=NAM03-BY2-obe.outbound.protection.outlook.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bkIK8-0001aE-Cw for qemu-devel@nongnu.org; Wed, 14 Sep 2016 18:06:32 -0400 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> From: Brijesh Singh Message-ID: <44c5f5f1-4697-6adb-4f4f-7203398bdd3b@amd.com> Date: Wed, 14 Sep 2016 17:06:23 -0500 MIME-Version: 1.0 In-Reply-To: <90efced4-3a77-d28b-e1fe-5a937bcf991b@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit 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: Paolo Bonzini , 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 Cc: brijesh.singh@amd.com On 09/14/2016 04:52 PM, Paolo Bonzini wrote: > > > On 14/09/2016 23:47, Brijesh Singh wrote: >> >> >> On 09/14/2016 04:00 PM, Paolo Bonzini wrote: >>> >>> >>> On 14/09/2016 22:59, Brijesh Singh wrote: >>>> I will look into hooking up the callback into ROM read/write ops. I was >>>> thinking about adding a new argument in >>>> cpu_physical_memory_write_rom_internal() >>>> >>>> void cpu_physical_memory_write_rom(AddressSpace *as, hwaddr addr, >>>> const uint8_t *buf, int len, >>>> WriteCB *cb) >>>> { >>>> .... >>>> ptr = qemu_map_ram_ptr(mr->ram_block, addr1); >>>> >>>> if (cb) >>>> cb(ptr, buf, len) >>>> else >>>> memcpy(ptr, buf, len) >>>> .... >>>> >>>> } >>>> >>>> In case of SEV, we pass a CB function pointer which calls SEV API's to >>>> encrypt memory. Does this make sense? >>> >>> I think a global as you have it in this series is just fine---just don't >>> hook it into address_space_read and address_space_write. >>> >> >> Actually in SEV RAM callback I check the Attrs, if attr.sev_debug flag >> set then use SEV debug command otherwise default to memcpy so that DMA >> and everything else works. I guest the main reason why i choose to hook >> this up in address_space_read/write was that I found that >> address_space_write and address_space_read is used in debug path. e.g >> >> cpu_memory_rw_debug >> address_space_rw >> address_space_write/read > > Right, but if you change this to a ROM hook only, cpu_memory_rw_debug > will go through cpu_physical_memory_write_rom instead. This will invoke > the hook properly, won't it? It will break -kernel unless fw_cfg DMA is > disabled, of course. > maybe I am missing something. here is what I see: int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr, uint8_t *buf, int len, int is_write) { ............ if (is_write) cpu_physical_memory_write_rom_internal() else address_space_rw() ..... } So looking at code, i have impression that write will go through the cpu_physical_memory_write_rom but the read will still go through address_space_rw which will eventually invoke address_space_read. Also when user tries to read or write to a physical address through qemu monitor then it will invoke cpu_physical_memory_rw which will eventually use address_space_write and address_space_read to read/write the guest memory.