From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33589) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTf6H-0007rM-Rm for qemu-devel@nongnu.org; Wed, 10 Feb 2016 19:27:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aTf6G-0000Br-Kh for qemu-devel@nongnu.org; Wed, 10 Feb 2016 19:27:13 -0500 Date: Thu, 11 Feb 2016 09:47:14 +1000 From: David Gibson Message-ID: <20160210234714.GH29288@voom.bne.redhat.com> References: <1455127752-17293-1-git-send-email-thuth@redhat.com> <1455127752-17293-5-git-send-email-thuth@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="pfTAc8Cvt8L6I27a" Content-Disposition: inline In-Reply-To: <1455127752-17293-5-git-send-email-thuth@redhat.com> Subject: Re: [Qemu-devel] [PATCH 4/4] hw/ppc/spapr: Implement the h_page_init hypercall List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thomas Huth Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org, laurent@vivier.eu --pfTAc8Cvt8L6I27a Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Feb 10, 2016 at 07:09:12PM +0100, Thomas Huth wrote: > This hypercall either initializes a page with zeros, or copies > another page. > According to LoPAPR, the i-cache of the page should also be > flushed when using H_ICACHE_INVALIDATE or H_ICACHE_SYNCHRONIZE, > but this is currently only done when running with TCG - assuming > the cache will be flushed with KVM anyway when switching back to > kernel / VM context. I don't think that's true. dcache and icache aren't usually flushed by kernel/user or even process context changes in Linux. Cache control instructions aren't priveleged so, I think to get this right you'd need a helper which does dcbst and icbi across the page. I'm pretty sure libc needs to do this at several points, but alas I don't think there's an exported function to do it. > The code currently also does not explicitely flush the data cache > with H_ICACHE_SYNCHRONIZE, since this either also should be done > when switching back to kernel / VM context (with KVM), or not matter > anyway (for TCG). >=20 > Signed-off-by: Thomas Huth > --- > hw/ppc/spapr_hcall.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 42 insertions(+) >=20 > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > index f14f849..91e703d 100644 > --- a/hw/ppc/spapr_hcall.c > +++ b/hw/ppc/spapr_hcall.c > @@ -387,6 +387,47 @@ static target_ulong h_set_xdabr(PowerPCCPU *cpu, sPA= PRMachineState *spapr, > return H_SUCCESS; > } > =20 > +static target_ulong h_page_init(PowerPCCPU *cpu, sPAPRMachineState *spap= r, > + target_ulong opcode, target_ulong *args) > +{ > + target_ulong flags =3D args[0]; > + target_ulong dest =3D args[1]; > + target_ulong src =3D args[2]; > + uint8_t buf[TARGET_PAGE_SIZE]; > + > + if (flags & ~(H_ICACHE_SYNCHRONIZE | H_ICACHE_INVALIDATE > + | H_COPY_PAGE | H_ZERO_PAGE)) { > + qemu_log_mask(LOG_UNIMP, "h_page_init: Bad flags (" TARGET_FMT_l= x "\n", > + flags); This should return H_PARAMETER as well as logging, surely? > + } > + > + if (!is_ram_address(spapr, dest) || (dest & ~TARGET_PAGE_MASK) !=3D = 0) { > + return H_PARAMETER; > + } > + > + if (flags & H_COPY_PAGE) { > + if (!is_ram_address(spapr, src) || (src & ~TARGET_PAGE_MASK) != =3D 0) { > + return H_PARAMETER; > + } > + cpu_physical_memory_read(src, buf, TARGET_PAGE_SIZE); > + } else if (flags & H_ZERO_PAGE) { > + memset(buf, 0, TARGET_PAGE_SIZE); > + } > + > + if (flags & (H_COPY_PAGE | H_ZERO_PAGE)) { > + cpu_physical_memory_write(dest, buf, TARGET_PAGE_SIZE); > + } Hmm, so this does 2 copies for an H_COPY_PAGE and a zero and a copy for H_ZERO_PAGE, which is going to be substantially slower than the caller might expect. > + if (flags & (H_ICACHE_SYNCHRONIZE | H_ICACHE_INVALIDATE)) { > + if (tcg_enabled()) { > + tb_flush(CPU(cpu)); > + } > + /* XXX: Flush data cache for H_ICACHE_SYNCHRONIZE? */ > + } > + > + return H_SUCCESS; > +} > + > #define FLAGS_REGISTER_VPA 0x0000200000000000ULL > #define FLAGS_REGISTER_DTL 0x0000400000000000ULL > #define FLAGS_REGISTER_SLBSHADOW 0x0000600000000000ULL > @@ -1046,6 +1087,7 @@ static void hypercall_register_types(void) > spapr_register_hypercall(H_SET_SPRG0, h_set_sprg0); > spapr_register_hypercall(H_SET_DABR, h_set_dabr); > spapr_register_hypercall(H_SET_XDABR, h_set_xdabr); > + spapr_register_hypercall(H_PAGE_INIT, h_page_init); > spapr_register_hypercall(H_SET_MODE, h_set_mode); > =20 > /* "debugger" hcalls (also used by SLOF). Note: We do -not- differen= ciate --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --pfTAc8Cvt8L6I27a Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWu8wCAAoJEGw4ysog2bOSBNYP/0sQ3ODmUj1QWsWCv7iHY6U6 VanXfltJWAXe3U4MWGkddvl+TxkcNMamg5XJhVlu+39eC4kR7u/6rsF8qG1LFePM hClBOdV0ysH+nfE0i4stV1U0sQxs+Q4MbYBPmDqziD4mywLRqlMkGDLaNuyK5luh nwAdeX02ZIzWRPa5xGhgKKn9XEkNWxDVWy0pCLUwOS0dkww2khWjJDG2t8Py9Iva 9ZJ48VTpit/Y3GvFKGoiJF3enZBxUnb1oFlZxgwIklv//+61/IqoICrIxumBIiNP ufH0gtslZQcMOjfalByt7aC5aA9Ae8HklquWPnSrBxpUPQHa+vbnE7t6ZsahtEGL 1PnPLZqwtuWFvuPALuT8Om4NpYeHr+Ad/sTk7vd5RxJNVCTf8siDtgfyQsGuuf6X fsIsy9LmAPgfT5ShHGuqaxny4iKbvhaxXQwTPb8ZCMjcPzBF8edNArvOaywK6RH/ gxtjosbfyncTO/zzCWoW+1uo4TLRMA6/BE8I8AQRPCqJMJn5yK2V+z1sV4lotuzC y20Wc1eNZJA6fMZhWFzzPHJ6lQ0ZPkWM0b64I3oTIe0jhnrVmtDC12rPvtacVNqb mL23wl/1ftH1LK73VYD3IU6R3hARMYF8mBAc6jRLsg3gM6clm9qk0ITgx/Y2bnbB OgVSKwROxzC0D9XKrvEL =uaZZ -----END PGP SIGNATURE----- --pfTAc8Cvt8L6I27a--