qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: BALATON Zoltan <balaton@eik.bme.hu>
Cc: Alexey Kardashevskiy <aik@ozlabs.ru>,
	qemu-ppc@nongnu.org, qemu-devel@nongnu.org
Subject: Re: [PATCH qemu v20] spapr: Implement Open Firmware client interface
Date: Wed, 2 Jun 2021 17:57:07 +1000	[thread overview]
Message-ID: <YLc507CyOVq403TM@yekko> (raw)
In-Reply-To: <653edbe1-f912-bc8f-ec7f-c4f069b0a5b9@eik.bme.hu>

[-- Attachment #1: Type: text/plain, Size: 13210 bytes --]

On Thu, May 27, 2021 at 02:42:39PM +0200, BALATON Zoltan wrote:
> On Thu, 27 May 2021, David Gibson wrote:
> > On Tue, May 25, 2021 at 12:08:45PM +0200, BALATON Zoltan wrote:
> > > On Tue, 25 May 2021, David Gibson wrote:
> > > > On Mon, May 24, 2021 at 12:55:07PM +0200, BALATON Zoltan wrote:
> > > > > On Mon, 24 May 2021, David Gibson wrote:
> > > > > > On Sun, May 23, 2021 at 07:09:26PM +0200, BALATON Zoltan wrote:
> > > > > > > On Sun, 23 May 2021, BALATON Zoltan wrote:
> > > > > > > > On Sun, 23 May 2021, Alexey Kardashevskiy wrote:
> > > > > > > > > One thing to note about PCI is that normally I think the client
> > > > > > > > > expects the firmware to do PCI probing and SLOF does it. But VOF
> > > > > > > > > does not and Linux scans PCI bus(es) itself. Might be a problem for
> > > > > > > > > you kernel.
> > > > > > > > 
> > > > > > > > I'm not sure what info does MorphOS get from the device tree and what it
> > > > > > > > probes itself but I think it may at least need device ids and info about
> > > > > > > > the PCI bus to be able to access the config regs, after that it should
> > > > > > > > set the devices up hopefully. I could add these from the board code to
> > > > > > > > device tree so VOF does not need to do anything about it. However I'm
> > > > > > > > not getting to that point yet because it crashes on something that it's
> > > > > > > > missing and couldn't yet find out what is that.
> > > > > > > > 
> > > > > > > > I'd like to get Linux working now as that would be enough to test this
> > > > > > > > and then if for MorphOS we still need a ROM it's not a problem if at
> > > > > > > > least we can boot Linux without the original firmware. But I can't make
> > > > > > > > Linux open a serial console and I don't know what it needs for that. Do
> > > > > > > > you happen to know? I've looked at the sources in Linux/arch/powerpc but
> > > > > > > > not sure how it would find and open a serial port on pegasos2. It seems
> > > > > > > > to work with the board firmware and now I can get it to boot with VOF
> > > > > > > > but then it does not open serial so it probably needs something in the
> > > > > > > > device tree or expects the firmware to set something up that we should
> > > > > > > > add in pegasos2.c when using VOF.
> > > > > > > 
> > > > > > > I've now found that Linux uses rtas methods read-pci-config and
> > > > > > > write-pci-config for PCI access on pegasos2 so this means that we'll
> > > > > > > probably need rtas too (I hoped we could get away without it if it were only
> > > > > > > used for shutdown/reboot or so but seems Linux needs it for PCI as well and
> > > > > > > does not scan the bus and won't find some devices without it).
> > > > > > 
> > > > > > Yes, definitely sounds like you'll need an RTAS implementation.
> > > > > 
> > > > > I plan to fix that after managed to get serial working as that seems to not
> > > > > need it. If I delete the rtas-size property from /rtas on the original
> > > > > firmware that makes Linux skip instantiating rtas, but I still get serial
> > > > > output just not accessing PCI devices. So I think it should work and keeps
> > > > > things simpler at first. Then I'll try rtas later.
> > > > > 
> > > > > > > While VOF can do rtas, this causes a problem with the hypercall method using
> > > > > > > sc 1 that goes through vhyp but trips the assert in ppc_store_sdr1() so
> > > > > > > cannot work after guest is past quiesce.
> > > > > > 
> > > > > > > So the question is why is that
> > > > > > > assert there
> > > > > > 
> > > > > > Ah.. right.  So, vhyp was designed for the PAPR use case, where we
> > > > > > want to model the CPU when it's in supervisor and user mode, but not
> > > > > > when it's in hypervisor mode.  We want qemu to mimic the behaviour of
> > > > > > the hypervisor, rather than attempting to actually execute hypervisor
> > > > > > code in the virtual CPU.
> > > > > > 
> > > > > > On systems that have a hypervisor mode, SDR1 is hypervisor privileged,
> > > > > > so it makes no sense for the guest to attempt to set it.  That should
> > > > > > be caught by the general SPR code and turned into a 0x700, hence the
> > > > > > assert() if we somehow reach ppc_store_sdr1().
> > > > > > 
> > > > > > So, we are seeing a problem here because you want the 'sc 1'
> > > > > > interception of vhyp, but not the rest of the stuff that goes with it.
> > > > > > 
> > > > > > > and would using sc 1 for hypercalls on pegasos2 cause other
> > > > > > > problems later even if the assert could be removed?
> > > > > > 
> > > > > > At least in the short term, I think you probably can remove the
> > > > > > assert.  In your case the 'sc 1' calls aren't truly to a hypervisor,
> > > > > > but a special case escape to qemu for the firmware emulation.  I think
> > > > > > it's unlikely to cause problems later, because nothing on a 32-bit
> > > > > > system should be attempting an 'sc 1'.  The only thing I can think of
> > > > > > that would fail is some test case which explicitly verified that 'sc
> > > > > > 1' triggered a 0x700 (SIGILL from userspace).
> > > > > 
> > > > > OK so the assert should check if the CPU has an HV bit. I think there was a
> > > > > #detine for that somewhere that I can add to the assert then I can try that.
> > > > > What I wasn't sure about is that sc 1 would conflict with the guest's usage
> > > > > of normal sc calls or are these going through different paths and only sc 1
> > > > > will trigger vhyp callback not affecting notmal sc calls?
> > > > 
> > > > The vhyp shouldn't affect normal system calls, 'sc 1' is specifically
> > > > for hypercalls, as opposed to normal 'sc' (a.k.a. 'sc 0'), and the
> > > > vhyp only intercepts the hypercall version (after all Linux on PAPR
> > > > certainly uses its own system calls, and hypercalls are active for the
> > > > lifetime of the guest there).
> > > > 
> > > > > (Or if this causes
> > > > > an otherwise unnecessary VM exit on KVM even when it works then maybe
> > > > > looking for a different way in the future might be needed.
> > > > 
> > > > What you're doing here won't work with KVM as it stands.  There are
> > > > basically two paths into the vhyp hypercall path: 1) from TCG, if we
> > > > interpret an 'sc 1' instruction we enter vhyp, 2) from KVM, if we get
> > > > a KVM_EXIT_PAPR_HCALL KVM exit then we also go to the vhyp path.
> > > > 
> > > > The second path is specific to the PAPR (ppc64) implementation of KVM,
> > > > and will not work for a non-PAPR platform without substantial
> > > > modification of the KVM code.
> > > 
> > > OK so then at that point when we try KVM we'll need to look at alternative
> > > ways, I think MOL OSI worked with KVM at least in MOL but will probably make
> > > all syscalls exit KVM but since we'll probably need to use KVM PR it will
> > > exit anyway. For now I keep this vhyp as it does not run with KVM for other
> > > reasons yet so that's another area to clean up so as a proof of concept
> > > first version of using VOF vhyp will do.
> > 
> > Eh, since you'll need to modify KVM anyway, it probably makes just as
> > much sense to modify it to catch the 'sc 1' as MoL's magic thingy.
> 
> I'm not sure how KVM works for this case so I also don't know why and what
> would need to be modified. I think we'll only have KVM PR working as newer
> POWER CPUs having HV (besides being rare among potential users) are probably
> too different to run the OSes that expect at most a G4 on pegasos2 so likely
> it won't work with KVM HV.

Oh, it definitely won't work with KVM HV.

> If we have KVM PR doesn't sc already trap so we
> could add MOL OSI without further modification to KVM itself only needing
> change in QEMU?

Uh... I guess so?

> I also hope that MOL OSI could be useful for porting some
> paravirt drivers from MOL for running Mac OS X on Mac emulation but I don't
> know about that for sure so I'm open to any other solution too.

Maybe.  I never know much about MOL to begin with, and anything I did
know was a decade or more ago so I've probably forgotten.

> For now I'm
> going with vhyp which is enough fot testing with TCG and if somebody wants
> KVM they could use he original firmware for now so this could be improved in
> a later version unless a simple solution is found before the freeze for 6.1.
> If we're in KVM PR what happens for sc 1 could that be used too so maybe
> what we have now could work?

Note that if you do go down the MOL path it wouldn't be that complex
to make a "vMOL" interface so you can use the same mechanism for KVM
and TCG.

> > > [...]
> > > > > > > I've tested that the missing rtas is not the reason for getting no output
> > > > > > > via serial though, as even when disabling rtas on pegasos2.rom it boots and
> > > > > > > I still get serial output just some PCI devices are not detected (such as
> > > > > > > USB, the video card and the not emulated ethernet port but these are not
> > > > > > > fatal so it might even work as a first try without rtas, just to boot a
> > > > > > > Linux kernel for testing it would be enough if I can fix the serial output).
> > > > > > > I still don't know why it's not finding serial but I think it may be some
> > > > > > > missing or wrong info in the device tree I generat. I'll try to focus on
> > > > > > > this for now and leave the above rtas question for later.
> > > > > > 
> > > > > > Oh.. another thought on that.  You have an ISA serial port on Pegasos,
> > > > > > I believe.  I wonder if the PCI->ISA bridge needs some configuration /
> > > > > > initialization that the firmware is expected to do.  If so you'll need
> > > > > > to mimic that setup in qemu for the VOF case.
> > > > > 
> > > > > That's what I begin to think because I've added everything to the device
> > > > > tree that I thought could be needed and I still don't get it working so it
> > > > > may need some config from the firmware. But how do I access device registers
> > > > > from board code? I've tried adding a machine reset method and write to
> > > > > memory mapped device registers but all my attempts failed. I've tried
> > > > > cpu_stl_le_data and even memory_region_dispatch_write but these did not get
> > > > > to the device. What's the way to access guest mmio regs from QEMU?
> > > > 
> > > > That's odd, cpu_stl() and memory_region_dispatch_write() should work
> > > > from board code (after the relevant memory regions are configured, of
> > > > course).  As an ISA serial port, it's probably accessed through IO
> > > > space, not memory space though, so you'd need &address_space_io.  And
> > > > if there is some bridge configuration then it's the bridge control
> > > > registers you need to look at not the serial registers - you'd have to
> > > > look at the bridge documentation for that.  Or, I guess the bridge
> > > > implementation in qemu, which you wrote part of.
> > > 
> > > I've found at last that stl_le_phys() works. There are so many of these that
> > > I never know when to use which.
> > > 
> > > I think the address_space_rw calls in vof_client_call() in vof.c could also
> > > use these for somewhat shorter code. I've ended up with
> > > stl_le_phys(CPU(cpu)->as, addr, val) in my machine reset methodbut I don't
> > > even need that now as it works without additional setup. Also VOF's memory
> > > access is basically the same as the already existing rtas_st() and co. so
> > > maybe that could be reused to make code smaller?
> > 
> > rtas_ld() and rtas_st() should only be used for reading/writing RTAS
> > parameters to and from memory.  Accessing IO shouldn't be done with
> > those.
> > 
> > For IO you probably want the cpu_st*() variants in most cases, since
> > you're trying to emulate an IO access from the virtual cpu.
> 
> I think I've tried that but what worked to access mmio device registers are
> stl_le_phys and similar that are wrappers around address_space_stl_*. But I
> did not mean that for rtas_ld/_st but the part when vof accessing the
> parameters passed by its hypercall which is memory access:
> 
> https://github.com/patchew-project/qemu/blob/patchew/20210520090557.435689-1-aik%40ozlabs.ru/hw/ppc/vof.c
> 
> line 893, and vof_client_call before that is very similar to what h_rtas
> does here:
> 
> https://git.qemu.org/?p=qemu.git;a=blob;f=hw/ppc/spapr_hcall.c;h=f25014afda408002ee1ec1027a0dd7a6025eca61;hb=HEAD#l639
> 
> and I also need to do the same for rtas in pegasos2 for which I'm just using
> ldl_be_phys for now but I wonder if we really need 3 ways to do the same or
> the rtas_ld/_st could be made more generic and reused here?

For your rtas implementation you could definitely re-use them.  For
the client call I'm a bit less confident, but if the in-guest-memory
structures are really the same, then it would make sense.

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2021-06-02  8:37 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-20  9:05 [PATCH qemu v20] spapr: Implement Open Firmware client interface Alexey Kardashevskiy
2021-05-20 21:59 ` BALATON Zoltan
2021-05-21  0:25   ` Alexey Kardashevskiy
2021-05-21  9:05     ` BALATON Zoltan
2021-05-21 19:57       ` BALATON Zoltan
2021-05-22  6:39         ` Alexey Kardashevskiy
2021-05-22 13:08           ` BALATON Zoltan
2021-05-23  3:47             ` Alexey Kardashevskiy
2021-05-23 12:12               ` BALATON Zoltan
2021-05-22  6:22       ` Alexey Kardashevskiy
2021-05-22 13:01         ` BALATON Zoltan
2021-05-22 15:02           ` BALATON Zoltan
2021-05-22 16:46             ` BALATON Zoltan
2021-05-23  3:41               ` Alexey Kardashevskiy
2021-05-23 12:02                 ` BALATON Zoltan
2021-05-23  3:31             ` Alexey Kardashevskiy
2021-05-23 11:24               ` BALATON Zoltan
2021-05-24  4:26                 ` Alexey Kardashevskiy
2021-05-24  5:40                   ` David Gibson
2021-05-24 11:56                     ` BALATON Zoltan
2021-05-23  3:20           ` Alexey Kardashevskiy
2021-05-23 11:19             ` BALATON Zoltan
2021-05-23 17:09               ` BALATON Zoltan
2021-05-24  6:01                 ` David Gibson
2021-05-24 10:55                   ` BALATON Zoltan
2021-05-24 12:46                     ` Alexey Kardashevskiy
2021-05-24 22:34                       ` BALATON Zoltan
2021-05-25  5:24                       ` David Gibson
2021-05-25  5:23                     ` David Gibson
2021-05-25 10:08                       ` BALATON Zoltan
2021-05-27  5:34                         ` David Gibson
2021-05-27 12:42                           ` BALATON Zoltan
2021-06-02  7:57                             ` David Gibson [this message]
2021-06-02 12:29                               ` BALATON Zoltan
2021-06-04  6:29                                 ` David Gibson
2021-06-04 13:59                                   ` BALATON Zoltan
2021-06-07  3:30                                     ` David Gibson
2021-06-07 22:54                                       ` BALATON Zoltan
2021-06-09  5:51                                         ` Alexey Kardashevskiy
2021-06-09 10:19                                           ` BALATON Zoltan
2021-06-06 22:21                                   ` BALATON Zoltan
2021-06-07  3:37                                     ` David Gibson
2021-06-07 22:20                                       ` BALATON Zoltan
2021-05-24 12:42                   ` BALATON Zoltan
2021-05-25  5:29                     ` David Gibson
2021-05-25  9:55                       ` BALATON Zoltan
2021-05-27  5:31                         ` David Gibson
2021-05-24  5:23   ` David Gibson
2021-05-24  9:57     ` BALATON Zoltan
2021-05-24 10:50       ` David Gibson
2021-05-29 18:10 ` BALATON Zoltan
2021-05-30 17:33 ` BALATON Zoltan
2021-05-31 13:07   ` BALATON Zoltan
2021-06-01 12:02     ` Alexey Kardashevskiy
2021-06-01 14:12       ` BALATON Zoltan
2021-06-04  6:21         ` David Gibson
2021-06-04 13:27           ` BALATON Zoltan
2021-06-07  3:02             ` David Gibson
2021-06-04  6:19   ` David Gibson
2021-06-04 13:50     ` BALATON Zoltan
2021-06-04 14:34       ` BALATON Zoltan
2021-06-07  3:05       ` David Gibson
2021-06-09  6:13         ` Alexey Kardashevskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YLc507CyOVq403TM@yekko \
    --to=david@gibson.dropbear.id.au \
    --cc=aik@ozlabs.ru \
    --cc=balaton@eik.bme.hu \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).