On 17.12.18 11:16, David Hildenbrand wrote:
> Looking at what the kernel does, looks like we should
> - Check for more errors
> - Try to execute !FORCED read if the FORCED one is unknown
> - Set the function code to a normal write
> - Set control_mask[2] to SCLP_VARIABLE_LENGTH_RESPONSE
>
> The kernel seems to set the function code to 0x80. Looking at where that
> comes from, turns out this is a "dump indicator" to produce more
> meaningful dumps. Looking for other function codes used in the kernel, I
> discovered 0x40, which is used for "single increment assign" to speed up
> memory hotplug. I was not able to find out what "control_mask[2] = 0x80"
> means.
We figured out what control_mask[2] = 0x80 is, so you can drop that.
Maybe rephrase that anyway and put it to the second to last paragraph.
>
> I guess we can just set the funcion code to 0 ("normal write). Let's
s/funcion/function/
> document the other bits in case anybody ever wonders what they mean.
> Rename SCLP_VARIABLE_LENGTH_RESPONSE to SCLP_CM2_VARIABLE_LENGTH_RESPONSE
> and move the SCLP function codes, mask bits and event buffer flags to
> dedicated sections in the header.
>
> Make sure that all other callers properly initialize the header
> fully (also inititalize control_mask and function_code).
s/inititalize/initialize/
>
> Tested-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
> Reviewed-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
> Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
> ---
>
> v1 -> v2:
> - Use define for control mask value 0x80
> - Rename SCLP_VARIABLE_LENGTH_RESPONSE to SCLP_CM2_VARIABLE_LENGTH_RESPONSE
> - Cleanup sclp.h header file a bit (move to sections)
> - Make sure control mask and function code are in a deterministic state
> when issuing sclp_service_call
>
> lib/s390x/sclp-ascii.c | 7 +++++++
> lib/s390x/sclp.c | 26 ++++++++++++++++++++++++--
> lib/s390x/sclp.h | 13 +++++++++----
> 3 files changed, 40 insertions(+), 6 deletions(-)
>
> diff --git a/lib/s390x/sclp-ascii.c b/lib/s390x/sclp-ascii.c
> index 893ca17..344a414 100644
> --- a/lib/s390x/sclp-ascii.c
> +++ b/lib/s390x/sclp-ascii.c
> @@ -38,6 +38,10 @@ static void sclp_set_write_mask(void)
> WriteEventMask *sccb = (void *)_sccb;
>
> sccb->h.length = sizeof(WriteEventMask);
> + sccb->h.function_code = SCLP_FC_NORMAL_WRITE;
> + sccb->h.control_mask[0] = 0;
> + sccb->h.control_mask[1] = 0;
> + sccb->h.control_mask[2] = 0;
I opted for a memset and I would prefer it here too, as I'll introduce
it anyway.
> sccb->mask_length = sizeof(unsigned int);
> sccb->receive_mask = SCLP_EVENT_MASK_MSG_ASCII;
> sccb->cp_receive_mask = SCLP_EVENT_MASK_MSG_ASCII;
> @@ -59,6 +63,9 @@ void sclp_print(const char *str)
>
> sccb->h.length = sizeof(WriteEventData) + len;
> sccb->h.function_code = SCLP_FC_NORMAL_WRITE;
> + sccb->h.control_mask[0] = 0;
> + sccb->h.control_mask[1] = 0;
> + sccb->h.control_mask[2] = 0;
> sccb->ebh.length = sizeof(EventBufferHeader) + len;
> sccb->ebh.type = SCLP_EVENT_ASCII_CONSOLE_DATA;
> sccb->ebh.flags = 0;
> diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c
> index 1d4a010..d04981d 100644
> --- a/lib/s390x/sclp.c
> +++ b/lib/s390x/sclp.c
> @@ -30,14 +30,36 @@ static void mem_init(phys_addr_t mem_end)
> phys_alloc_init(freemem_start, mem_end - freemem_start);
> }
>
> +static void sclp_read_scp_info(ReadInfo *ri, int length)
> +{
> + unsigned int commands[] = { SCLP_CMDW_READ_SCP_INFO_FORCED,
> + SCLP_CMDW_READ_SCP_INFO };
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(commands); i++) {
> + ri->h.length = length;
> + ri->h.function_code = SCLP_FC_NORMAL_WRITE;
> + ri->h.control_mask[0] = 0;
> + ri->h.control_mask[1] = 0;
> + ri->h.control_mask[2] = SCLP_CM2_VARIABLE_LENGTH_RESPONSE;
Same here.
> +
> + if (sclp_service_call(commands[i], ri))
> + break;
> + if (ri->h.response_code == SCLP_RC_NORMAL_READ_COMPLETION)
> + return;
> + if (ri->h.response_code != SCLP_RC_INVALID_SCLP_COMMAND)
> + break;
> + }
> + report_abort("READ_SCP_INFO failed");
> +}
> +
> void sclp_memory_setup(void)
> {
> ReadInfo *ri = (void *)_sccb;
> uint64_t rnmax, rnsize;
> int cc;
>
> - ri->h.length = SCCB_SIZE;
> - sclp_service_call(SCLP_CMDW_READ_SCP_INFO_FORCED, ri);
> + sclp_read_scp_info(ri, SCCB_SIZE);
>
> /* calculate the storage increment size */
> rnsize = ri->rnsize;
> diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h
> index 21d482b..629e9e2 100644
> --- a/lib/s390x/sclp.h
> +++ b/lib/s390x/sclp.h
> @@ -68,14 +68,19 @@
> #define SCLP_RC_EVENT_BUFFER_SYNTAX_ERROR 0x73f0
> #define SCLP_RC_INVALID_MASK_LENGTH 0x74f0
>
> -/* Service Call Control Block (SCCB) and its elements */
> +/* SCLP control mask bits */
> +#define SCLP_CM2_VARIABLE_LENGTH_RESPONSE 0x80
>
> -#define SCCB_SIZE 4096
> +/* SCLP function codes */
> +#define SCLP_FC_NORMAL_WRITE 0
> +#define SCLP_FC_SINGLE_INCREMENT_ASSIGN 0x40
> +#define SCLP_FC_DUMP_INDICATOR 0x80
>
> -#define SCLP_VARIABLE_LENGTH_RESPONSE 0x80
> +/* SCLP event buffer flags */
> #define SCLP_EVENT_BUFFER_ACCEPTED 0x80
>
> -#define SCLP_FC_NORMAL_WRITE 0
> +/* Service Call Control Block (SCCB) and its elements */
> +#define SCCB_SIZE 4096
>
> typedef struct SCCBHeader {
> uint16_t length;
>
Attachment:
signature.asc
Description: OpenPGP digital signature
