* Cornelia Huck <coh...@redhat.com> [2018-02-27 18:32:51 +0100]:
> The license text currently specifies "any version" of the GPL. It
> is unlikely that GPL v1 was ever intended; change this to the
> standard "or any later version" text.
>
> Cc: Dong J
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.
AFAI read the code, this series does not hurt the vritual devices, which
always have fixed path information in SCHIB.
The strange thing is, the chp 0.00 is both virtual and non-virtual with
mix use of virtual and non-virtual devices. And any existing problem is
not caused or enhanced by this work. If we can not fix the problem
without MCSS support or chp re-modelling in the near future, we should
document the problem and the effect.
I will do some test on this.
>
>>
>> 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 <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 m
o
>> 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
* Halil Pasic <pa...@linux.vnet.ibm.com> [2018-01-16 16:57:13 +0100]:
>
>
> On 01/15/2018 09:59 AM, Dong Jia Shi wrote:
> > * Halil Pasic <pa...@linux.vnet.ibm.com> [2018-01-12 19:10:20 +0100]:
> >
> >>
> >>
> >> On 01/11/2018 04:0
* Pierre Morel <pmo...@linux.vnet.ibm.com> [2018-01-15 11:21:47 +0100]:
> On 15/01/2018 09:57, Dong Jia Shi wrote:
> >* Cornelia Huck <coh...@redhat.com> [2018-01-11 11:54:22 +0100]:
> >
> >>On Thu, 11 Jan 2018 04:04:18 +0100
> >>Dong Jia Shi <
* Cornelia Huck <coh...@redhat.com> [2018-01-15 13:24:22 +0100]:
> On Mon, 15 Jan 2018 10:50:00 +0100
> Pierre Morel <pmo...@linux.vnet.ibm.com> wrote:
>
> > On 11/01/2018 04:04, Dong Jia Shi wrote:
> > > This introduces a new region for vfio-ccw to provide s
* Halil Pasic <pa...@linux.vnet.ibm.com> [2018-01-12 19:10:20 +0100]:
>
>
> On 01/11/2018 04:04 AM, Dong Jia Shi wrote:
> > What are still missing, thus need to be offered in the next version are:
> > - I/O termination and FSM state handling if currently we
* Cornelia Huck <coh...@redhat.com> [2018-01-11 11:54:22 +0100]:
> On Thu, 11 Jan 2018 04:04:18 +0100
> Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> wrote:
>
> > Hi Folks,
> >
> > Background
> > ==
> >
> > Some d
* Cornelia Huck <coh...@redhat.com> [2018-01-11 15:16:59 +0100]:
Hi Conny,
> On Thu, 11 Jan 2018 04:04:19 +0100
> Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> wrote:
>
> > This introduces a new region for vfio-ccw to provide subchannel
> > information for user sp
are at it, touch one line of the comment around by
adding white space to make it aligned.
Signed-off-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
---
hw/s390x/css.c | 83 -
hw/s390x/s390-ccw.c | 20 +++
hw/vfio
path information update, we only do update if we were signaled.
We sets scsw.pno and pmcw.pnom to indicate path related event
for a guest. This is a bit lazy, but it works.
Signed-off-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
---
hw/s390x/css.c | 14 +--
hw/s390x/s390
with the schib likeness.
Signed-off-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
---
hw/vfio/ccw.c | 24 +++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
index 16713f2c52..e673695509 100644
--- a/hw/vfio/ccw.c
+++ b/hw/vfio
This adds channel path related event handler for vfio-ccw.
This also signals userland when there is a chp event.
Signed-off-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
---
drivers/s390/cio/vfio_ccw_drv.c | 51 +
drivers/s390/cio/vfio_ccw_fsm.c
path information
out once got the eventfd signal, and do further process.
Signed-off-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
---
hw/vfio/ccw.c | 92 +--
1 file changed, 70 insertions(+), 22 deletions(-)
diff --git a/hw/vfio/ccw.
Hi Folks,
This is the QEMU couterpart for the "basic channel path event handling" series.
For more information, please refer to the kernel counterpart.
Dong Jia Shi (5):
vfio: linux-headers update for vfio-ccw
vfio/ccw: get schib region info
vfio/ccw: get irq info and set
This is a placeholder for a linux-headers update.
Signed-off-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
---
linux-headers/linux/vfio.h | 2 ++
linux-headers/linux/vfio_ccw.h | 6 ++
2 files changed, 8 insertions(+)
diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/
This introduces a new irq for vfio-ccw to provide channel path
related event for userland.
Signed-off-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
---
drivers/s390/cio/vfio_ccw_ops.c | 29 +
drivers/s390/cio/vfio_ccw_private.h | 2 ++
include/uapi/linux/
This introduces a new region for vfio-ccw to provide subchannel
information for user space.
Signed-off-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
---
drivers/s390/cio/vfio_ccw_fsm.c | 21 ++
drivers/s390/cio/vfio_ccw_ops.c | 79 +++--
d
PIM PAM POM CHPIDs
--
0.0.3f3f 0.0.0002 3390/0c 3990/e9 f0 f0 ff 42434445
#Notice: PAM changed again.
Dong Jia Shi (3):
vfio: ccw: introduce schib region
vfio: ccw: introduce channel path irq
vfio: ccw: handle chp event
drivers/s390/cio/vfio_
* Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> [2017-12-05 15:43:00 +0800]:
> * Cornelia Huck <coh...@redhat.com> [2017-12-04 17:11:24 +0100]:
>
> [...]
>
> This one looks good to me too, so:
> Reviewed-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
>
>
* Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> [2017-12-05 15:43:00 +0800]:
> * Cornelia Huck <coh...@redhat.com> [2017-12-04 17:11:24 +0100]:
>
> [...]
>
> This one looks good to me too, so:
> Reviewed-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
>
BT
* Cornelia Huck <coh...@redhat.com> [2017-12-04 17:11:24 +0100]:
[...]
This one looks good to me too, so:
Reviewed-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
>
> (...)
>
> > > Looks sane. We should put a note into the 2.12 changelog as well.
> &
cssid, regardless whether virtual"
> >
> > extra space -^
> >
>
> Nod.
>
> >> +" or not (read only, always true)",
>
> Do we need "." here ^ ?
>
> >> +NULL);
> >> }
> >>
> >> static const TypeInfo virtual_css_bridge_info = {
> >
> > Looks reasonable. If this works as expected, I'll squash it into the
> > previous patch.
> >
>
> I've just asked Shalini to verify the libvirt perspective.
>
> Supposed we verify this works as expected, I read I don't have to spin
> a v2 and you are going to fix the issues found yourself. Right?
Supposed this works as expected, you have my r-b. ;)
--
Dong Jia Shi
* Halil Pasic <pa...@linux.vnet.ibm.com> [2017-12-01 15:31:34 +0100]:
[...]
No comment for the message part.
The code looks good to me. So after squashing with patch #2:
Reviewed-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
> ---
> hw/s390x/3270-ccw.c| 2 +-
ces of the test results between w and w/o this patch are in
> > the "squashing off" cases. I think these are things that we can accept.
>
> Yes, I think that makes sense. If you want reliable migration, you need
> to be specific with your ids. I'd just don't want us to break things
> explicitly.
>
Fair enough. Got the message.
--
Dong Jia Shi
* Halil Pasic <pa...@linux.vnet.ibm.com> [2017-11-29 17:30:15 +0100]:
>
>
> On 11/29/2017 12:47 PM, Cornelia Huck wrote:
> > On Wed, 29 Nov 2017 16:17:35 +0800
> > Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> wrote:
> >
> >> * Halil Pasic <p
* Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> [2017-11-29 16:17:35 +0800]:
[...]
> T6. squashing off + explicit given id
> qemu-system-s390x: vmstate: get_nullptr expected VMS_NULLPTR_MARKER
> qemu-system-s390x: Failed to load s390_css:css
> qemu-system-s390x: error w
0.0003/virtio-rng' 0
qemu-system-s390x: load of migration failed: Invalid argument
[Fail due to busid mismatch.]
T8. squashing on + explicit given id
Succeed.
Notice:
The differences of the test results between w and w/o this patch are in
the "squashing off" cases. I think these are things that we can accept.
[...]
--
Dong Jia Shi
* Cornelia Huck <coh...@redhat.com> [2017-11-27 13:58:16 +0100]:
> On Mon, 27 Nov 2017 10:20:56 +0800
> Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> wrote:
>
> > * Halil Pasic <pa...@linux.vnet.ibm.com> [2017-11-24 17:39:04 +0100]:
> >
> > >
&g
gt;
> Proposal 1: Use a device attribute to show that the device can be put
> anywhere. This approach feels wrong to me (the non-restriction is not a
> property of the individual device, but of the whole css resp. the
> machine), so I would continue to veto this.
>
> Proposal 2:
ignore the operation of setting it's value to
true.
> >
> > (Unless we simply make this a "default cssid" prop after all - then it
> > would be more than just a simple indication for libvirt...)
> >
>
> We are now talking about the "cssid-unrestricted" property. The default
> cssid is not something I would like to do any time soon.
--
Dong Jia Shi
>
Regarding to test coverage, this mainly covers the cds_read. What we
want would be surely more than this. So to extend the coverage, we need
to design more modes (aka, test cases), right?
If nobody disagrees with the basic idea of this series, I can start a
review then. ;)
[...]
--
Dong Jia Shi
* Cornelia Huck <coh...@redhat.com> [2017-11-14 11:50:14 +0100]:
Hallo Conny,
After spending some time, just some updates for this one.
> On Tue, 14 Nov 2017 16:25:47 +0800
> Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> wrote:
>
> > Dear Conny,
> >
> > G
or QEMU cmd line users. And we are wondering if there is some
way to improve it.
Thanks,
--
Dong Jia Shi
NC|SCSW_FCTL_HALT_FUNC|SCSW_FCTL_CLEAR_FUNC))
> >>> {
> >>> - ret = -EBUSY;
> >>> -goto out;
> >>> +return IOINST_CC_STATUS_PRESENT;
> >>> }
> >>
> >> ... but here you change -EBUSY (which got mapped to CC=2) to
> >> CC_STATUS_PRESENT which means CC=1. So that's a change in behavior. i.e.
> >> this is either a bug, or you should update the patch description with a
> >> justification for this change in behavior.
> >
> > Indeed, that's a bug. We still want cc 2.
> >
>
> Agree, it is a bug. It was OK in v1 but was already buggy in
> v2.
>
> Conny can you fix this up as well please?
>
> Thanks in advance!
>
I saw Conny fixed this in her branch. So:
Reviewed-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
--
Dong Jia Shi
sch)) {
> +setcc(cpu, 3);
Ditto.
> +return;
> }
> -setcc(cpu, cc);
> +setcc(cpu, css_do_hsch(sch));
> }
>
> static int ioinst_schib_valid(SCHIB *schib)
> --
> 2.13.5
>
Reviewed-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
--
Dong Jia Shi
uint64_t reg1)
> --
> 2.13.5
>
If we agree to replace 3 with IOINST_CC_NOT_OPERATIONAL, maybe Conny
could fix it. Or we can do that in a following cleanup patch?
W/ or w/o the fix, for now:
Reviewed-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
--
Dong Jia Shi
-ENODEV:
> -cc = 3;
> -break;
> -case -EBUSY:
> -cc = 2;
> -break;
> -case 0:
> -cc = 0;
> - break;
> -default:
> -cc = 1;
> -break;
> +if (!sch || !css_subch_visible(sch)) {
> +setcc(cpu, 3);
?:
s/3/IOINST_CC_NOT_OPERATIONAL/
This also applies to other alike cases.
> +return;
> }
> -setcc(cpu, cc);
> +setcc(cpu, css_do_xsch(sch));
> }
>
> void ioinst_handle_csch(S390CPU *cpu, uint64_t reg1)
> --
> 2.13.5
>
Reviewed-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
--
Dong Jia Shi
iles changed, 75 insertions(+), 126 deletions(-)
>
Agree for what already planned to fix when applying.
LGTM:
Reviewed-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
--
Dong Jia Shi
* Cornelia Huck <coh...@redhat.com> [2017-10-18 11:53:10 +0200]:
> On Wed, 18 Oct 2017 16:23:47 +0800
> Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> wrote:
>
> > * Halil Pasic <pa...@linux.vnet.ibm.com> [2017-10-17 18:19:20 +0200]:
>
t; --
> 2.13.5
>
With what has been pointed out by Conny and Thomas addressed:
Reviewed-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
--
Dong Jia Shi
series (#2-#7), I didn't notice
any obvious problem during my fio test. So for the vfio-ccw related
part:
Tested-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
[...]
--
Dong Jia Shi
* Halil Pasic <pa...@linux.vnet.ibm.com> [2017-10-11 12:54:51 +0200]:
>
>
> On 10/11/2017 05:47 AM, Dong Jia Shi wrote:
> > * Halil Pasic <pa...@linux.vnet.ibm.com> [2017-10-04 17:41:39 +0200]:
> >
> >> Simplify the error handling of the SSC
* Halil Pasic <pa...@linux.vnet.ibm.com> [2017-10-10 12:06:23 +0200]:
>
>
> On 10/10/2017 10:13 AM, Dong Jia Shi wrote:
> > * Halil Pasic <pa...@linux.vnet.ibm.com> [2017-10-04 17:41:39 +0200]:
> >
> > [...]
> >
> >> diff --git a
return (IOInstEnding){.cc = 0};
> }
> -
> -return region->ret_code;
> }
>
> static void vfio_ccw_reset(DeviceState *dev)
> diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h
> index 66916b6546..2116c6b3c7 100644
> --- a/include/hw/s390x/css.h
> +++ b/include/hw/s390x/css.h
[...]
> @@ -197,13 +208,14 @@ SubchDev *css_find_subch(uint8_t m, uint8_t cssid,
> uint8_t ssid,
> uint16_t schid);
> bool css_subch_visible(SubchDev *sch);
> void css_conditional_io_interrupt(SubchDev *sch);
> +
Extra change.
> int css_do_stsch(SubchDev *sch, SCHIB *schib);
> bool css_schid_final(int m, uint8_t cssid, uint8_t ssid, uint16_t schid);
> int css_do_msch(SubchDev *sch, const SCHIB *schib);
[...]
--
Dong Jia Shi
+IOInstEnding s390_ccw_cmd_request(SubchDev *sch)
> {
> -S390CCWDeviceClass *cdc = S390_CCW_DEVICE_GET_CLASS(data);
> +S390CCWDeviceClass *cdc = S390_CCW_DEVICE_GET_CLASS(sch->driver_data);
>
> -if (cdc->handle_request) {
> -return cdc->handle_request(orb, scsw, data);
> -} else {
> -return -ENOSYS;
> +if (!cdc->handle_request) {
> +return (IOInstEnding){.cc = 1};
Same consideration as the last comment.
> }
> +return cdc->handle_request(sch);
> }
>
> static void s390_ccw_get_dev_info(S390CCWDevice *cdev,
[...]
--
Dong Jia Shi
nnel_work(SubchDev *sch)
> {
> -if (sch->do_subchannel_work) {
> -return sch->do_subchannel_work(sch);
> -} else {
> +if (!sch->do_subchannel_work) {
> return -EINVAL;
> }
> + g_assert(sch->curr_status.scsw.ctrl & SCSW_CTRL_MASK_FCTL);
> +return sch->do_subchannel_work(sch);
> }
>
> static void copy_pmcw_to_guest(PMCW *dest, const PMCW *src)
With the fix of the message:
Reviewed-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
--
Dong Jia Shi
* Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> [2017-09-26 15:48:56 +0800]:
[...]
> > >
> > > Tried to test with the following method:
> > > 1. Start g1 (first level guest on kvm a host) with a virtio blk device
> > >defined:
> > > -drive
&
* Halil Pasic <pa...@linux.vnet.ibm.com> [2017-09-25 12:57:31 +0200]:
[restored Cc:]
>
>
> On 09/25/2017 09:31 AM, Dong Jia Shi wrote:
> > * Cornelia Huck <coh...@redhat.com> [2017-09-08 11:59:50 +0200]:
> >
> >> On Fri, 8 Sep 2017 11:21:57 +0200
* Cornelia Huck <coh...@redhat.com> [2017-09-21 10:54:02 +0200]:
> On Thu, 21 Sep 2017 16:45:47 +0800
> Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> wrote:
>
> > * Cornelia Huck <coh...@redhat.com> [2017-09-07 10:08:17 +0200]:
> >
> > [...]
>
* Cornelia Huck <coh...@redhat.com> [2017-09-08 11:59:50 +0200]:
> On Fri, 8 Sep 2017 11:21:57 +0200
> Halil Pasic <pa...@linux.vnet.ibm.com> wrote:
>
> > On 09/08/2017 05:41 AM, Dong Jia Shi wrote:
> > > Let' me summarize here, in case I misunderstan
* Cornelia Huck <coh...@redhat.com> [2017-09-08 12:02:38 +0200]:
> On Fri, 8 Sep 2017 11:41:00 +0800
> Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> wrote:
>
> > I think the difficult part is in the Qemu side. For either A or B, in
> > Qemu, we'd need to return a cc
d-off-by: Halil Pasic <pa...@linux.vnet.ibm.com>
> ---
> hw/s390x/css.c | 114
> -
> 1 file changed, 113 insertions(+), 1 deletion(-)
>
LGTM:
Reviewed-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
[...]
--
Dong Jia Shi
gt;cda_orig = ccw->cda;
> ccw_dstream_rewind(cds);
> diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h
> index 078356e94c..69b374730e 100644
> --- a/include/hw/s390x/css.h
> +++ b/include/hw/s390x/css.h
> @@ -87,6 +87,7 @@ typedef struct CcwData
injects the I/O interrupt in a
synchronzing manner. And this causes g1's I/O interrupt handler getting
the interrupt and then signaling the Qemu instance on g1 with the I/O
result, even before return of the pwrite().
But, using gdb on the kvm host, I do see several ssch successfully
executed. I will dig the root reason, and see if there is some way to
fix the issue.
--
Dong Jia Shi
* Halil Pasic <pa...@linux.vnet.ibm.com> [2017-09-20 13:13:01 +0200]:
>
>
> On 09/20/2017 10:33 AM, Cornelia Huck wrote:
> > On Wed, 20 Sep 2017 15:42:38 +0800
> > Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> wrote:
> >
> >> * Halil Pasic <p
ornelia Huck wrote:
> >>> On Wed, 20 Sep 2017 15:42:38 +0800
> >>> Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> wrote:
> >>>
> >>>> * Halil Pasic <pa...@linux.vnet.ibm.com> [2017-09-19 20:27:45 +0200]:
> >
> >>>>&g
* Halil Pasic <pa...@linux.vnet.ibm.com> [2017-09-20 13:02:59 +0200]:
>
>
> On 09/20/2017 10:25 AM, Cornelia Huck wrote:
> > On Wed, 20 Sep 2017 15:47:51 +0800
> > Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> wrote:
> >
> >> * Halil Pasic <p
/css.h
> index 078356e94c..69b374730e 100644
> --- a/include/hw/s390x/css.h
> +++ b/include/hw/s390x/css.h
> @@ -87,6 +87,7 @@ typedef struct CcwDataStream {
> #define CDS_F_MIDA 0x02
> #define CDS_F_I2K 0x04
> #define CDS_F_C64 0x08
> +#define CDS_F_FMT 0x10 /* CCW format-1 */
> #define CDS_F_STREAM_BROKEN 0x80
> uint8_t flags;
> uint8_t at_idaw;
> --
> 2.13.5
>
--
Dong Jia Shi
void *buff, int len,
> + CcwDataStreamOp op)
> +{
> +uint64_t bsz = ccw_ida_block_size(cds->flags);
> +int ret = 0;
> + uint16_t cont_left, iter_len;
> +const bool idaw_fmt2 = cds->flags & CDS_F_C64;
> +bool ccw_fmt1 = cds->flags & CDS_F_FMT;
Use 'const bool' either? Although I doubt the value of using const here.
;)
> +
> +ret = cds_check_len(cds, len);
> +if (ret <= 0) {
> +return ret;
> +}
[...]
--
Dong Jia Shi
pa...@linux.vnet.ibm.com>
> Reviewed-by: Pierre Morel<pmo...@linux.vnet.ibm.com>
> ---
> hw/s390x/virtio-ccw.c | 157
> +++---
> 1 file changed, 46 insertions(+), 111 deletions(-)
>
Reviewed-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
[...]
--
Dong Jia Shi
ot be used as is_write */
Looks good to me.
> +} CcwDataStreamOp;
> +
>
[...]
--
Dong Jia Shi
AWs).
>
> Reference:
> PoP pages 16-25 and 16-26 "Invalid IDAW or MIDAW Addre" and
> "Invalid Data Address".
>
> How shall we proceed?
>
> Halil
>
> >>>>
> >>>>> +return -EINVAL; /* channel program check */
> >>>>> +
> >>>>> +}
> >>>>> +ret = address_space_rw(_space_memory, idaw_addr,
> >>>>> + MEMTXATTRS_UNSPECIFIED, (void *)
> >>>>> ,
> >>>>> + sizeof(idaw.fmt1), false);
> >>>>> +cds->cda = be64_to_cpu(idaw.fmt1);>>>>> +}
> >>>>> +++(cds->at_idaw);
> >>>>> +if (ret != MEMTX_OK) {
> >>>>> +/* assume inaccessible address */
> >>>>> +return -EINVAL; /* channel program check */
> >>>>> +
> >>>>> +}
> >>>>> +return 0;
> >>>>> +}
--
Dong Jia Shi
^
> >>>> Nit.
> >>>>
> >> Maybe checkpatch wanted it this way. My memories are blurry.
> > I'd just leave it like that, tbh.
>
> Yes, if I remove the space before the } checkpatch.pl complains.
>
> Going to keep it as is for v3.
My bad. :-/
>
> Halil
--
Dong Jia Shi
* Halil Pasic <pa...@linux.vnet.ibm.com> [2017-09-19 15:30:29 +0200]:
>
>
> On 09/19/2017 11:06 AM, Cornelia Huck wrote:
> > On Tue, 19 Sep 2017 11:37:30 +0800
> > Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> wrote:
> >
> >> * Halil Pasic <p
grees.
> > (The 3270 emulation authors are busy of other stuff these days. :()
>
> Generally, yes I agree. If it's trivial I will probably do it myself
> for v2. I need to do a v2 anyway.
>
> Halil
I saw you are working on this. So leave it to you. ;)
--
Dong Jia Shi
gt; +}
> +cont_left = bsz;
> +} while (true);
> +return ret;
> +err:
> +cds->flags |= CDS_F_STREAM_BROKEN;
> +return ret;
> +}
> +
> void ccw_dstream_init(CcwDataStream *cds, CCW1 const *ccw, ORB const *orb)
> {
> /*
> @@ -835,7 +942,7 @@ void ccw_dstream_init(CcwDataStream *cds, CCW1 const
> *ccw, ORB const *orb)
> if (!(cds->flags & CDS_F_IDA)) {
> cds->op_handler = ccw_dstream_rw_noflags;
> } else {
> -assert(false);
> +cds->op_handler = ccw_dstream_rw_ida;
> }
> }
>
> --
> 2.13.5
>
Generally, the logic looks fine to me.
--
Dong Jia Shi
MEMTXATTRS_UNSPECIFIED, NULL);
> +ccw_dstream_read_buf(>cds, , 4);
^
A magic number? O:)
> +be16_to_cpus();
> +be16_to_cpus();
> if (ccw.count < len + revinfo.length ||
> (check_len && ccw.count > len + revinfo.length)) {
> ret = -EINVAL;
> --
> 2.13.5
>
Otherwise, looks good.
--
Dong Jia Shi
m_write_buf(>cds, _id, len);
> +sch->curr_status.scsw.count = ccw_dstream_residual_count(>cds);
> ret = 0;
> break;
> }
> --
> 2.13.5
>
Reviewed-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
--
Dong Jia Shi
^^
Nit.
> +}
[...]
With or w/o responses the nit picks:
Reviewed-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
--
Dong Jia Shi
to cc0.
2. We need to signal Qemu to fetch the IRB we generated with the checks.
All of these are doable I think.
Any comment for the above thoughts?
--
Dong Jia Shi
* Cornelia Huck <coh...@redhat.com> [2017-09-07 12:52:20 +0200]:
> On Thu, 7 Sep 2017 12:21:50 +0200
> Halil Pasic <pa...@linux.vnet.ibm.com> wrote:
>
> > On 09/07/2017 10:08 AM, Cornelia Huck wrote:
> > > On Thu, 7 Sep 2017 15:31:09 +0800
> > > Don
ostly likely that the vfio-ccw driver processed the ccw chains
wrongly. (Actually I can not think of any other reason.)
This is option 1.
c. ccwchain_fetch_idal
When we find that an IDAW contents an invalid address
This is option 2.
> >>
> >> I would expect option 2 to be handled differently (kernel provides the
> >> SCSW) though.
> >>
> >> Regards,
> >> Halil
> >>
> >
> >
--
Dong Jia Shi
* Cornelia Huck <coh...@redhat.com> [2017-09-06 13:25:38 +0200]:
> On Wed, 6 Sep 2017 16:27:20 +0800
> Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> wrote:
>
> > * Halil Pasic <pa...@linux.vnet.ibm.com> [2017-09-05 19:20:43 +0200]:
> >
> > >
&g
...
Some days ago, one of my colleague tried the emulated 3270 passing
through. She stucked with the problem that the level 1 kvm host handling
a senseid IDAL ccw as a Direct ccw.
Maybe I could try to pass through a virtio ccw device. I don't think of
any obvious problem that would lead to fail. Any comment?
--
Dong Jia Shi
e go with that option?
>
> If converting the reporting to a device status is straightforward
> enough, let's do that. I'm fine with postponing this and waiting for a
> real fix as well (I don't really have spare cycles to actually write
> vfio-ccw code currently...)
>
I can do this after this series.
[...]
--
Dong Jia Shi
mmer is trying to figure out why things
> don't work (why are we getting the unit exception)? I think that's
> best remedied with producing something for the log (maybe a warning
> with warn_report which states that the implementation vfio-ccw requires
> the given flags).
Fine with me.
[...]
--
Dong Jia Shi
that's
> the case we will never reach the code in question.
Agree.
> So I suppose we can do better.
As the comment said, I'm (still) in the state of 'wondering'.
>
> Adding Ren. @Ren: Do you agree with my analysis. If you do,
> I could come up with a proposal what to do -- after some reading.
If you have a better idea, and time, why not? ;)
>
> Regards,
> Halil
--
Dong Jia Shi
Let's use a macro for the ERC (error recover code) when generating a
Channel Subsystem Event-information pending CRW (channel report word).
While we are at it, let's also add all other ERCs.
Signed-off-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
Reviewed-by: Halil Pas
A successful completion of rchp should signal a solicited channel path
initialized CRW (channel report word), while the current implementation
always generates an un-solicited one. Let's fix this.
Reported-by: Halil Pasic <pa...@linux.vnet.ibm.com>
Signed-off-by: Dong Jia Shi
update.
Patch #2:
Rename the new added parameter to more speaking name -- solicited.
Dong Jia Shi (2):
s390x/css: use macro for event-information pending error recover code
s390x/css: generate solicited crw for rchp completion signaling
hw/s390x/css.c| 16 ++--
includ
* Halil Pasic <pa...@linux.vnet.ibm.com> [2017-08-01 17:16:37 +0200]:
>
>
> On 08/01/2017 09:57 AM, Dong Jia Shi wrote:
> [..]
> > --- a/hw/s390x/css.c
> > +++ b/hw/s390x/css.c
> > @@ -1745,10 +1745,10 @@ int css_do_rchp(uint8_t cssid, uint8_t chpid)
> &g
* Halil Pasic <pa...@linux.vnet.ibm.com> [2017-08-01 17:24:10 +0200]:
>
>
> On 08/01/2017 09:57 AM, Dong Jia Shi wrote:
> > Let's use a macro for the ERC (error recover code) when generating a
> > Channel Subsystem Event-information pending CRW (channel repo
A successful completion of rchp should signal a solicited channel path
initialized CRW (channel report word), while the current implementation
always generates an un-solicited one. Let's fix this.
Reported-by: Halil Pasic <pa...@linux.vnet.ibm.com>
Signed-off-by: Dong Jia Shi
This series is trying to:
1. clear up ERC related code
2. bugfix for channel path related CRW generation
Change log
--
v1->v2:
Patch #1:
Add all ERCs.
Commit message update.
Patch #2:
Rename the new added parameter to more speaking name -- solicited.
Dong Jia Shi (2):
s390x/
Let's use a macro for the ERC (error recover code) when generating a
Channel Subsystem Event-information pending CRW (channel report word).
While we are at it, let's also add all other ERCs.
Signed-off-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
---
hw/s390x/css.c| 2 +-
i
* Cornelia Huck <coh...@redhat.com> [2017-08-01 09:24:20 +0200]:
> On Tue, 1 Aug 2017 10:29:10 +0800
> Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> wrote:
>
> > * Cornelia Huck <coh...@redhat.com> [2017-07-31 13:13:02 +0200]:
> >
> > > On Mon, 31 J
* Cornelia Huck <coh...@redhat.com> [2017-07-31 13:13:02 +0200]:
> On Mon, 31 Jul 2017 11:51:37 +0800
> Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> wrote:
>
> > * Cornelia Huck <coh...@redhat.com> [2017-07-27 13:59:10 +0200]:
> >
> > > On Thu, 27 J
* Cornelia Huck <coh...@redhat.com> [2017-07-31 10:54:47 +0200]:
> On Fri, 28 Jul 2017 23:50:48 +0800
> Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> wrote:
>
> > * Cornelia Huck <coh...@redhat.com> [2017-07-28 13:53:01 +0200]:
>
> > > > > You're
* Halil Pasic <pa...@linux.vnet.ibm.com> [2017-07-31 14:30:32 +0200]:
>
>
> On 07/31/2017 01:13 PM, Cornelia Huck wrote:
> > On Mon, 31 Jul 2017 11:51:37 +0800
> > Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> wrote:
> >
> >> * Cornelia Huc
* Cornelia Huck <coh...@redhat.com> [2017-07-31 10:41:47 +0200]:
> On Mon, 31 Jul 2017 09:46:17 +0800
> Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> wrote:
>
> > * Cornelia Huck <coh...@redhat.com> [2017-07-28 14:58:19 +0200]:
>
> > > Exposing real ch
* Cornelia Huck <coh...@redhat.com> [2017-07-27 13:59:10 +0200]:
> On Thu, 27 Jul 2017 03:54:18 +0200
> Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> wrote:
>
> > When a channel path is hot plugged into a CSS, we should generate
> > a channel path initialized CRW (
to support a guest OS that
> does not also run under LPAR, I'd prefer that way.
>
My poor English... Sorry, I don't undersatnd the last sentence...
[...]
--
Dong Jia Shi
hpid))
> > chp_new(chpid);
> > chsc_chp_online(chpid);
> >
> > Notice:
> > At the very beginning, I replaced CRW_ERC_IPARM with CRW_ERC_INIT. But
> > Sebstian Ott suggested:
> > "I don't know of a machine that actually implements a CRW
> > at all when a chpid is configured online on the SE/HMC.
> >
> > Because of potential regressions I don't want to remove CRW_ERC_IPARM
> > here. I'm good with adding CRW_ERC_INIT though."
>
> Yeah, that makes sense, especially with the confusing state channel
> path machine check handling is in from the architecture side.
>
> >
> > >
> > > I'll double check with how I'd interpret the PoP today.
> > >
> > Thanks.
>
> I have read through the PoP and the outcome is a bit disappointing.
> Much of it is a bit vague. I still think that you can err on the side
> of overindication, though.
>
I agree. Unless somebody tells me it's forbidden by the PoP explicitly,
or it will break Linux guest from working properly, I will would this as
the way to go.
--
Dong Jia Shi
* Cornelia Huck <coh...@redhat.com> [2017-07-27 11:46:03 +0200]:
> On Thu, 27 Jul 2017 03:54:15 +0200
> Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> wrote:
>
> > This series is trying to:
> > 1. clear up CRW related code.
> > 2. generate the right cha
-void css_queue_crw(uint8_t rsc, uint8_t erc, int chain, uint16_t rsid);
> > +void css_queue_crw(uint8_t rsc, uint8_t erc, int s, int chain, uint16_t
> > rsid);
> > void css_generate_sch_crws(uint8_t cssid, uint8_t ssid, uint16_t schid,
> > int hotplugged, int add);
> > void css_generate_chp_crws(uint8_t cssid, uint8_t chpid);
>
> Otherwise, patch looks good.
Thanks.
So, I only need to s/s/solicited for the new version of this one?
>
--
Dong Jia Shi
error, facility not init */
#define CRW_ERC_PERRI0x07 /* perm. error, facility init */
#define CRW_ERC_PMOD 0x08 /* installed parameters modified */
Want the comment or not? I like them.
> >
> > #define CRW_RSC_SUBCH 0x3
> > #define CRW_RSC_CHP 0x4
>
--
Dong Jia Shi
00644
> --- a/hw/s390x/css.c
> +++ b/hw/s390x/css.c
> @@ -795,6 +795,10 @@ static int css_interpret_ccw(SubchDev *sch, hwaddr
> ccw_addr,
> if (!ccw_addr) {
> return -EIO;
> }
> +/* Check doubleword aligned and 31 or 24 (fmt 0) bit addressable. */
> +if
> > +if (ccw.flags || ccw.count) {
> > +/* We have already sanitized these if fmt 0. */
> > ret = -EINVAL;
> > break;
> > }
>
> Thanks, applied (with tweaked comment).
>
> Dong Jia: I've added your R-b, please let me know if that's not ok.
Yes, please. That's ok.
(Just cann't help to miss the chance to reply to you ;)
--
Dong Jia Shi
1 - 100 of 352 matches
Mail list logo
