Re: [PATCH RFC 2/2] vfio-ccw: support for halt/clear subchannel

[Halil Pasic]

On 06/12/2018 03:56 PM, Pierre Morel wrote:
> > So,ÂwhatÂareÂyouÂproposing?ÂBeingÂmoreÂspecificÂandÂstatingÂthatÂthe
> > scswÂisÂnotÂnecessarilyÂaÂrealÂscsw,ÂbutÂmerelyÂaÂvehicleÂforÂsendingÂa
> > command?ÂOrÂkeepingÂitÂasÂitÂisÂnowÂforÂssch,ÂandÂaddingÂaÂsecond
> > interfaceÂforÂhsch/cschÂ(andÂmaybeÂrsch,Âmsch,Â...)?
>
>
> IÂsaidÂnoÂradicalÂsurgery,ÂbutÂafterÂthinkingÂmoreÂaboutÂit...
> IÂamÂnotÂsure.
>
> Let'sÂexplainÂthis:
>
> I see 2 ways to proceed but my favorite is definitively to introduce versioning.
>
>
> WayÂ1)
>
> ThisÂwasÂtheÂwayÂIÂfirstÂthoughtÂabout.
> WeÂkeepÂtheÂexistingÂIOÂRegionandÂstructures,ÂandÂareÂmore
> specificÂbyÂstatingÂthatÂtheÂio_regionÂisÂaÂcommandÂregionÂduringÂwrite
> entryÂandÂaÂstatusÂregionÂduringÂinterruptÂhandling:
> ThisÂallowÂusÂtoÂuseÂtheÂ3ÂbitsÂofÂtheÂFCTLÂfieldÂofÂtheÂSCSWÂtoÂpass
> commandsÂtoÂtheÂkernelÂandÂstayÂbackwardÂcompatible.
> TheÂFCTLÂfieldÂhasÂ3ÂbitsÂ=>ÂweÂcanÂhaveÂ8Âcommands.
>
> PRO:ÂsmallÂchange
>
> CONTRA:ÂThisÂisÂstillÂconfusing,ÂweÂdoÂnotÂreallyÂsolveÂthis
>  ÂÂÂÂÂÂÂÂunclarityÂproblemÂsinceÂQEMUÂviewÂ/ÂdocumentationÂand
>  ÂÂÂÂÂÂÂÂLinuxÂviewÂ/ÂdocumentationÂdifferÂorÂweÂupdateÂQEMU.
>  ÂÂÂÂÂÂÂÂMoreoverÂthisÂdoesÂnotÂallowÂforÂlongÂtermÂextensions
>  ÂÂÂÂÂÂÂÂand/orÂre-design.

I'm not really in favor of way 1. Conie's point with RSCH is a good one.
And IMHO it speaks for a new interface for commands. Squeezing the RSCH
command into the SCSW does not seem to be a good idea. Considering your
proposal with the 3 bits, we could do something like: if in FCTL the
start and the clear and the halt bits are set then we postulate that is
request for a resume. But that would be quite confusing, and we would end
up re-defining the semantic of the scsw_area -- in respect to what is
documented Documentation/s390/vfio-ccw.txt, and also what is intuitive
based on the uapi header.

> WayÂ2)
>
> WeÂuseÂtheÂdeviceÂVFIOÂversioningÂusingÂtheÂcapabilityÂchainÂtoÂadvertise
> aÂnewÂinterface.
>
> ThisÂtheÂpreferredÂway,ÂitÂisÂsane,ÂallowsÂforÂtheÂuserlandÂbackward
> compatibilityÂandÂallowsÂtoÂhaveÂaÂnewÂcommandÂinterface,Âextensible
> forÂfutureÂuse.
>
> InÂthisÂcaseÂweÂcanÂextendÂtheÂinterfaceÂtoÂbeÂanyÂkindÂweÂwant
> inÂaÂnextÂversion,ÂpwriteÂwithÂnewÂio_region,ÂmmapÂonÂnew
> IOÂregions,ÂstatusÂregion...
>
> PRO:ÂExtensibleÂandÂalsoÂgoesÂinÂtheÂVFIOÂINFOÂextensionÂdirection
>  ÂÂÂÂÂproposedÂbyÂAlex


Sounds much better. I imagine the coexistence of old and new like this.
Both the old and the new QEMU would supply the the SCSW area with the old
documented semantics -- the SCSW of the virtual subchannel. But with the
new version an explicit command would be supplied via the command region
(also for  SSCH). Maybe the SCSW can still end up being useful for
something in the kernel module too (maybe there are some  optimization
that can be done based on the QEMU SCSW).


> CONTRA:ÂIÂseeÂnoneÂouterÂmoreÂworkÂtoÂdoÂ(butÂisÂitÂaÂproblem?)
The problem I see is that designing a good interface usually hard.
I could help with review, but I don't have the resources to commit
to more than that.

> ====================
>
> ExtraÂargumentationÂforÂversioningÂsupport
>
>
> SupposeÂaÂfutureÂimplementationÂwithÂ4ÂmappedÂregionsÂlike
> theÂfollowing:
>
> -ÂAÂStatusÂregionÂ(ROÂupdatedÂasÂresultÂofÂcommandÂandÂIRQ)
>
> -ÂAÂcommandÂregionÂ(WOÂwhereÂtheÂuserÂsendÂitsÂcommands)
>  ÂÂuserlandÂwriteÂhereÂtoÂtriggerÂIOÂ(quiteÂasÂcurrently)
>
> -ÂAÂCCWÂprogramÂregionÂ(RWÂwhereÂtheÂCCWÂchainÂisÂhandled)
>  ÂÂmostÂhandlingÂdoneÂfromÂuserspace,ÂlastÂtranslationsÂinÂkernel,
>  ÂÂdoubleÂbuffering...
>
> -ÂAÂperformanceÂ/ÂmeasurementÂ/ÂstatisticsÂregionÂ(RO)
>  ÂÂThisÂisÂupdatedÂasynchronouslyÂbyÂhardwareÂand/orÂdriver
>
> ThisÂisÂpurelyÂtheoreticalÂandÂweÂdoÂnotÂneedÂtoÂdoÂallÂatÂonce
> butÂifÂweÂwantÂtoÂextendÂtheÂimplementationÂwithoutÂproblems
> andÂcontinueÂbackwardÂcompatibilityÂtheÂversioningÂandÂcapability
> handlingÂisÂaÂmust.

I'm not sure about this.


Regards,
Halil
[..]