From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33144) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZG2II-0001Wp-Hb for qemu-devel@nongnu.org; Fri, 17 Jul 2015 05:51:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZG2ID-0003c4-Hk for qemu-devel@nongnu.org; Fri, 17 Jul 2015 05:51:02 -0400 Received: from smtp.ispras.ru ([83.149.199.79]:56459) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZG2ID-0003Y0-4S for qemu-devel@nongnu.org; Fri, 17 Jul 2015 05:50:57 -0400 Message-ID: <55A8CFFF.30101@ispras.ru> Date: Fri, 17 Jul 2015 12:50:55 +0300 From: =?UTF-8?B?0JXRhNC40LzQvtCyINCS0LDRgdC40LvQuNC5?= MIME-Version: 1.0 References: <1437035704-11299-1-git-send-email-real@ispras.ru> <1437035704-11299-4-git-send-email-real@ispras.ru> <55A773C1.6060400@redhat.com> <55A78C99.5040504@ispras.ru> <55A79117.2020300@redhat.com> <55A7C2A9.6040902@ispras.ru> <55A7EF57.4080306@redhat.com> In-Reply-To: <55A7EF57.4080306@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 3/3] PAM: make PAM emulation closer to documentation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org Cc: Kirill Batuzov , "Michael S. Tsirkin" 16.07.2015 20:52, Paolo Bonzini =D0=BF=D0=B8=D1=88=D0=B5=D1=82: > > > On 16/07/2015 16:41, =D0=95=D1=84=D0=B8=D0=BC=D0=BE=D0=B2 =D0=92=D0=B0=D1= =81=D0=B8=D0=BB=D0=B8=D0=B9 wrote: >> The main problem is rendering memory tree to FlatView. > > I don't believe it's necessary to render a memory tree to the FlatView. > You can use existing AddressSpaces. > >>> + /* Read from RAM and write to PCI */ >>> + memory_region_init_io(&pam->region[1], OBJECT(dev), &pam_ops, pa= m, >>> + "pam-r-ram-w-pci", size); >>> >>> This can be done with memory_region_set_readonly on the RAM region. = You >>> need to set mr->ops in order to point to pam_ops; for a first proof o= f >>> concept you can just set the field directly. >> The idea is to read directly from system RAM region and to write >> to PCI using I/O (because I do not see another way to emulate >> access type driven redirection with existent API). If we create RAM >> and make it read only then new useless RAM block will be created. > > Don't create RAM; modify the existing one. > >> It is useless because of ram_addr of new region will be set to >> one within system RAM block. Hence, cleaner way is to create I/O regio= n. > > You can use the existing RAM region and modify its properties (i.e. > toggle mr->readonly) after setting special mr->ops. The special mr->op= s > will be used for writes when mr->readonly =3D true. The existing RAMs are ether whole pc.ram or pc.rom and pc.bios beyond PCI. So, I think we have not invade them. Also, we only need the small=20 subrange to be read only. Moreover, if -pflash is used then there is ROM device instead of pc.bios with its own mr->ops. You have suggested to create AddressSpace for PCI below. It is flexible solution which is transparent for existing regions. I suggests to create AddressSpace for RAM subtree too. I think, both AS have to be PAM implementation private for complete transparency for another QEMU parts. I will try it in next patch version. > >>> Writes to the PCI memory space can use the PCI address space, with >>> address_space_st*. >> There is no PCI AddressSpace (only MemoryRegion). But >> address_space_st* requires AddressSpace as argument. > > Then create one with address_space_init. > > However, can the guest see the difference between "real" mode 1 and the > "fake" mode 1 that QEMU implements? Perhaps mode 1 can be left as is. Yes, guest can. If -pfalsh is used then BIOS becomes a programmable ROM device. Then guest can switch to mode 1 and copy data by reading and writing it at same location. After that guest can switch to mode 0 (or 3) and copy data to some another place. Then it switches to mode 3 (or 0) and compares the data. I just check it with SeaBIOS and original PAM adding code listed below to fw/shadow.c: __make_bios_writable_intel. dprintf(1, "PAM mode 1 test begin\n"); unsigned *m =3D (unsigned *) BUILD_ROM_START; pci_config_writeb(bdf, pam0 + 1, 0x33); *m =3D 0xdeafbeef; pci_config_writeb(bdf, pam0 + 1, 0x11); volatile unsigned t =3D *m; *m =3D t; pci_config_writeb(bdf, pam0 + 1, 0x33); t =3D *m; pci_config_writeb(bdf, pam0 + 1, 0x00); unsigned t2 =3D *m; dprintf(1, "t =3D 0x%x, t2 =3D 0x%x\n", t, t2); dprintf(1, "PAM mode 1 test end\n"); The output is: PAM mode 1 test begin t =3D 0xdeafbeef, t2 =3D 0x0 PAM mode 1 test end With new PAM the output is: PAM mode 1 test begin t =3D 0xdeafbeef, t2 =3D 0xdeafbeef PAM mode 1 test end Note BUILD_ROM_START is 0xc0000 which is pc.rom with write permission according to info mtree output. The -pflash is not needed. > >>> + /* Read from PCI and write to RAM */ >>> + memory_region_init_io(&pam->region[2], OBJECT(dev), &pam_ops, pa= m, >>> + "pam-r-pci-w-ram", size); >>> >>> Here you cannot run code from ROM, so it can be a pure MMIO region. >>> Reads can use address_space_ld*, while writes can use >>> memory_region_get_ram_ptr. >> >> Even in this mode it is possible for code to be executed from ROM. Thi= s >> can happen when particular PCI address is within ROM device connected >> to PCI bus. > > If it's just for pc.rom and isa-bios, introduce a new function > pam_create_pci_region that creates pc.rom with > memory_region_init_rom_device. The mr->ops can write to RAM (mode 2) o= r > discard the write (mode 0). > > They you can make pc.rom 256K instead of 128K, and instead of an alias, > you can manually copy the last 128K of the BIOS into the last 128K of > pc.rom. > > Some adjustment will be necessary in order to support migration (perhap= s > creating two 128K regions pc.rom and pc.rom.mirror), but for a proof of > concept the above should be enough. I will try solution based on address spaces as stated above. It is=20 cleaner and more generic. > > Paolo > Vasily