On Wed, 2022-03-23 at 18:45 +0100, Claudio Imbrenda wrote:
> On Wed, 23 Mar 2022 18:03:20 +0100
> Nico Boehr <nrb@xxxxxxxxxxxxx> wrote:
>
[...]
> > +
> > +static int memisset(void *s, int c, size_t n)
>
> function should return bool..
Sure, changed.
[...]
> > +static void test_store_adtl_status(void)
> > +{
> >
[...]
> > +
> > + report_prefix_push("unaligned");
> > + smp_cpu_stop(1);
> > +
> > + cc = smp_sigp(1, SIGP_STORE_ADDITIONAL_STATUS,
> > + (unsigned long)&adtl_status + 256, &status);
> > + report(cc == 1, "CC = 1");
> > + report(status == SIGP_STATUS_INVALID_PARAMETER, "status =
> > INVALID_PARAMETER");
>
> and check again that nothing has been written to
Oh, thanks. Fixed.
[...]
> > +static void test_store_adtl_status_unavail(void)
> > +{
> > + uint32_t status = 0;
> > + int cc;
> > +
> > + report_prefix_push("store additional status unvailable");
> > +
> > + if (have_adtl_status()) {
> > + report_skip("guarded-storage or vector facility
> > installed");
> > + goto out;
> > + }
> > +
> > + report_prefix_push("not accepted");
> > + smp_cpu_stop(1);
> > +
> > + cc = smp_sigp(1, SIGP_STORE_ADDITIONAL_STATUS,
> > + (unsigned long)&adtl_status, &status);
> > +
> > + report(cc == 1, "CC = 1");
> > + report(status == SIGP_STATUS_INVALID_ORDER,
> > + "status = INVALID_ORDER");
> > +
>
> I would still check that nothing is written even when the order is
> rejected
Won't hurt, added.
[...]
> > +static void restart_write_vector(void)
> > +{
> > + uint8_t *vec_reg;
> > + /*
> > + * vlm handles at most 16 registers at a time
> > + */
>
> this comment can /* go on a single line */
OK
[...]
> > + /*
> > + * i+1 to avoid zero content
> > + */
>
> same /* here */
OK, changed.
[...]
> > +static void __store_adtl_status_vector_lc(unsigned long lc)
> > +{
> > + uint32_t status = -1;
> > + struct psw psw;
> > + int cc;
> > +
> > + report_prefix_pushf("LC %lu", lc);
> > +
> > + if (!test_facility(133) && lc) {
> > + report_skip("not supported, no guarded-storage
> > facility");
> > + goto out;
> > + }
>
> I think this ^ should not be there at all
It must be. If we don't have guarded-storage only LC 0 is allowed:
"When the guarded-storage facility is not installed, the
length and alignment of the MCESA is 1024 bytes.
When the guarded-storage facility is installed, the
length characteristic (LC) in bits 60-63 of the
MCESAD specifies the length and alignment of the
MCESA as a power of two"
See below for the reason why we don't have gs here.
[...]
> > diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
> > index 1600e714c8b9..843fd323bce9 100644
> > --- a/s390x/unittests.cfg
> > +++ b/s390x/unittests.cfg
> > @@ -74,9 +74,29 @@ extra_params=-device diag288,id=watchdog0 --
> > watchdog-action inject-nmi
> > file = stsi.elf
> > extra_params=-name kvm-unit-test --uuid 0fb84a86-727c-11ea-bc55-
> > 0242ac130003 -smp 1,maxcpus=8
> >
> > -[smp]
> > +[smp-kvm]
> > file = smp.elf
> > smp = 2
> > +accel = kvm
> > +extra_params = -cpu host,gs=on,vx=on
> > +
> > +[smp-no-vec-no-gs-kvm]
> > +file = smp.elf
> > +smp = 2
> > +accel = kvm
> > +extra_params = -cpu host,gs=off,vx=off
> > +
> > +[smp-tcg]
> > +file = smp.elf
> > +smp = 2
> > +accel = tcg
> > +extra_params = -cpu qemu,vx=on
>
> why not gs=on as well?
I am not an expert in QEMU CPU model, but it seems to me TCG doesn't
support it.
