* Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> [2018-01-30 11:37:43 +0800]:
Damn.. Please just ignore the above mail. It's not the right draft. > Halil Pasic <pa...@linux.vnet.ibm.com> writes: > > Hi Halil, > > As you may noticed, Conny replied to this thread on my mail. Some of her > comments there could answer your questions. If that applies, I will just > say "See Conny's mail" in the following, and you can reply to that mail, > so we can consolidate further discussion. > > >>> * When a channel path malfunctions a CRW is generated and a machine > >>> check channel report pending is generated. Same goes for > >>> channel paths popping up (on HW level). Should not these get > >>> propagated? > >> AFAIR, channel path related CRW is not that reliable. I mean it seems > >> that it's not mandatory to generate CRWs for channel path malfunctions. > >> AFAIU, it depneds on machine models. For example, we won't get > >> CRW_ERC_INIT for a "path has come" event on many models I believe. And > >> that's why nobody noticed the misuse of CRW_ERC_IPARM and CRW_ERC_INIT > >> in the CRW handling logic for channel path CRWs. > >> Ref: > >> 2daace78a8c9 s390: chp: handle CRW_ERC_INIT for channel-path status change > >> > > > > Honestly, I forgot about this discussion. I've refreshed my memories by > > now, but I could not find why is it supposed to be architecturally OK > > to loose CRWs when for instance a chp goes away. > > > >> So my understanding for this is: it really a design decision for us to > >> propagate all of the channel path CRWs, or we just use other ways (like > >> using PNO and PNOM as this series shows). > > > > From what I read, CRWs did not seem optional. > > I wonder what should I read in order to change my mind. I'm not > > talking about the hardware in the wild -- but that could be buggy > > hardware. > > > Ah, can you point out the chapter that says CRWs is mandatory? > > AFAIU, PoP doesn't say, for example, chp gone will lead to a CRW, but it > only says when we got a chp gone CRW it means a chp has been gone. > > [See Conny's mail pls, and we can discuss there.] > > >> > >> Of course, CRW propagation is good to have. But as a discussion result > >> of a former thread, that is something we can consider only after we have > >> a good channel path re-modelling. That is the problem. We can try to > >> trigger CRW event in some work of machine checks handling enhancement > >> later. > >> > >>> * Kind of instead of the CRW you introduce a per device interrupt > >>> for channel path events on the qemu/kvm boundary. AFAIU for a chp > >>> event you do an STSCH for each (affected?) subchannel > >> Until here, yes and right. > >> > >>> and generate an irq. Right? Why is this a good idea. > >> This is not right. This series does not generate an irq for the guest. > > > > You misunderstood this. The word 'irq' this sentence is a short version > > of 'per device interrupt for channel path events on the qemu/kvm boundary' > > form the previous sentence. It's not an irq injected into the guest but > > a notification (which you call irq in '[RFC PATCH 2/3] vfio: ccw: > > introduce channel path irq') for QEMU. > > > I see now. I misunderstood. > > >> In QEMU, when it gets the notification of a channel path status change > >> event, it read the newest SCHIB from the schib region, and update the > >> SCHIB (patch related parts) for the virtual subchannel. The guest will > >> get the new SCHIB whenever it calls STSCH then. > > > > We are in agreement. I just wanted to say, if let's say 42 vfio-ccw devices > > are using the same chp and it goes away, you will generate 42 notifications. > Yes, and this is the only way we can do for now, since there is no good > chp model, we can't use CRWs to consolidate the notifications... Once we > had the model and we can use CRW to indicate chp event, this could be > easily removed and changed to that. > > Once I had a version which leverages chp CRWs. But since we had > agreement that CRW related code change will not be acceptable in the > QEMU side before we have chp re-modelling doen, I changed to this PNO > and PNOM implementation. > > > > >> > >> I think this is a good idea, because: > >> 1. This is complies the Linux implementation. Path status change could > >> be noticed by Linux. > >> 2. This provides enhancement with a small work. On the contrary, channel > >> path re-modelling needs a lot of work. > > > > I thing your answer is based on the misunderstanding explained above. > I see now. > > > > > My idea was to be lazy in a different way. Instead of being lazy about > > reading the subsection, I was wondering why not do an STSCH in the host > > as a response to one being done in the guest. > Hey, my way is only an upgrade version of your way, and is a bit more > lazy than yours. ;) > > > > > That means: if there is no activity on the passed trough devices, nothing > > needs to be done. (Except maybe the CRWs). > > > > The things aren't observable by the guest if it does not do STSCH anyway. > Nod. This is the idea that this series is based on. > > > > > > >> > >>> * SCHIB.PMCW provides path information relevant for a given device. > >>> This information is retrieved using store subchannel. Is your series > >>> sufficient for making store subchannel work properly (correct and > >>> reasonably up to date)? > >> Introducing a SCHIB region is the basis of further STSCH hanlding work. > >> This series depends on it, and only focuses on the update of the channel > >> path related parts of it. And for these parts, this work I think is > >> basically correct (more review comments are surely to be welcomed). > >> > > > > Please elaborate on that basically. Or are those actually just correct > > in your opinion?! > > > >> For the rest parts, this does not change anything than what have. As > >> replied to Conny in other mail of this thread: I think, if the other > >> fields are handled by QEMU well, then we don't need to trigger update > >> events for them. Currently I don't find things that need extra trigger. > >> > > > > Fair. > > > >>> Regarding PMCW it's supposed to get updated when io functions are > >>> preformed by the css, AFAIR. Is that right? > >> I think so. And also for some other cases, for example, when we > >> configure on/off a channel path. > >> > > > > Sorry, I ended up confusing opm with pom. That would have been illegal > > (as Connie has pointed out recently) because it can only change > > only if the path is tried for IO. > > > > > >>> If yes what are the implications? Are the manipulations you do on > >>> some PMCW fields architecturally correct? > >> If "architecturally correct" means what guest sees/senses is all obey to > >> the PoP, then I think so. We don't have to emulate the internal > >> behaviors (which could not be seen by OS) of CSS exactly when we > >> emulate, if the emulation does not impact on what Linux guest will see, > >> right? > >> > >> I mean, the time point we update the PMCW does not has to be in the time > >> slot of io function performance. The guest would still have to get the > >> updated information through the calls of STSCH. We only need to provide > >> the updaetd result to the guest by handling STSCH well. And that's why > >> we say we do that lazily. > >> > >> Nothing different (added value) will be seen by the guest, comparing > >> with the current lazy implementation I think.> > >>> * The one thing I would very much appreciate as an user of vfio, > >>> and should in my understanding be the user story of this series > >>> (and the qemu counterpart of course) is the following. If my device > >>> (that is IO operation) failed because no path could be found on > >>> which the device is accessible, I would like to be able to identify > >>> that. Preferably the same way I would do for an LPAR guest. Is this > >>> series accomplishing that? > >> With this the guest could lazily identify that. But since we have no > >> machine check propagation yet, for those cases which generate CRWs, it > >> might not be the same way for an LPAR guest I think. > >> > > > > This was a yes/no question. I can't interpret your answer neither as > > yes or as no. Maybe a little clarification: I'm talking about a > > recent linux guest. > > > >>> * Why not just do proper STSCH for vfio-ccw? > >> I only did the interested parts (path related). For the other parts, the > >> current handling is enough, no? Anyway, after we have the schib region, > >> we can do further STSCH handling any time later we want then. > >> > > > > I mean, your approach works on the premise, that the host will notify > > QEMU each time the content of the SCHIB (area) changes (modulo stuff) so > > it can do an update in QEMU, and give the guest an up to date virtualized > > SCHIB when it asks for it executing the STSCH instruction. > > > > I was asking myself. Why not instead while interpreting STSCH do a > > STSCH on the host device (that is passed through), and virtualize the > > result as necessary. > > > > It occurred to me if nothing simpler. > > > >>> * Shouldn't we virtualize CHPIDs? > >> Nobody would say no for this. :) The problem is that this is something > >> big, and it's not a short-term goal in my current working project... I > >> really want to deliver a minimal function set in the near future > >> firstly. If the current working does not hurt, can we defer channel path > >> re-modelling and CHPIDs virtualization? > > > > The problem is, I'm not sure it does not hurt. For instance because of > > the question below. I would also prefer having a fair idea of what > > we need for the envisioned (complete) solution before introducing > > kernel interfaces (to avoid: pity in the end we need something > > different, and resulting cluttered interface). > > > >> > >>> What if we have a clash? Lets say my dasd is only accessible via chp > >>> 0.00 in the host. Currently we have a problem there, or (as the only > >>> path would end up being ignored)? > >> You mean the clash that sharing path 0.00 between a physical dasd and > >> the virtio ccw devices? The problem I saw is in the checking of the > >> chpid.is_virtual in css_do_rchp(). For a virtual chp, we should generate > >> INIT CRWs; while for a non-virtual one, we shouldn't. If the chp is > >> shared with virtual and non-virtual device, I don't know what to do > >> then... > >> > >> I don't think we can fix this before we re-modelled the channel path. > >> But this is neither introduced nor aggravated by this series, right? > > > > I'm not sure about the later. What prevents the guest from believing > > the (to use your terminology shared) chp 0.00 has become unavailable > > if 0.00 becomes unavailable in the host? > > > > Would that adversely affect the virtual devices? It would at > > least look strange. > > > >> > >> We'd have to document this either I think. > >> > >>> Note: this is another unpleasant effect of not having MCSSE in the > >>> guest. The original design was to have a separate css for vfio, and > >>> then we would not have this kind of a problem. (Virtualization of > >>> chps would still be good for migration.) > >> Nod. BTW, I don't say that I do not want channel path virtualization in > >> the long run. It's just I want a minimal function set more currently > >> from what I'm focusing perspective. > >> > >> THANKS for these insightful questions! > >> > > -- > Dong Jia Shi > > -- Dong Jia Shi
