LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v4 14/25] nvdimm/ocxl: Add support for Admin commands
From: Dan Williams @ 2020-04-02  6:41 UTC (permalink / raw)
  To: Alastair D'Silva
  Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
	Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
	Rob Herring, Dave Jiang, linux-nvdimm, Aneesh Kumar K . V,
	Krzysztof Kozlowski, Anju T Sudhakar, Mahesh Salgaonkar,
	Andrew Donnellan, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
	Cédric Le Goater, Thomas Gleixner, Hari Bathini, Linux MM,
	Greg Kroah-Hartman, Linux Kernel Mailing List, Vishal Verma,
	Frederic Barrat, Paul Mackerras, Andrew Morton, linuxppc-dev,
	David S. Miller
In-Reply-To: <20200327071202.2159885-15-alastair@d-silva.org>

On Sun, Mar 29, 2020 at 10:23 PM Alastair D'Silva <alastair@d-silva.org> wrote:
>
> Admin commands for these devices are the primary means of interacting
> with the device controller to provide functionality beyond the load/store
> capabilities offered via the NPU.
>
> For example, SMART data, firmware update, and device error logs are
> implemented via admin commands.
>
> This patch requests the metadata required to issue admin commands, as well
> as some helper functions to construct and check the completion of the
> commands.
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
>  drivers/nvdimm/ocxl/main.c              |  65 ++++++
>  drivers/nvdimm/ocxl/ocxlpmem.h          |  50 ++++-
>  drivers/nvdimm/ocxl/ocxlpmem_internal.c | 261 ++++++++++++++++++++++++
>  3 files changed, 375 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/nvdimm/ocxl/main.c b/drivers/nvdimm/ocxl/main.c
> index be76acd33d74..8db573036423 100644
> --- a/drivers/nvdimm/ocxl/main.c
> +++ b/drivers/nvdimm/ocxl/main.c
> @@ -217,6 +217,58 @@ static int register_lpc_mem(struct ocxlpmem *ocxlpmem)
>         return 0;
>  }
>
> +/**
> + * extract_command_metadata() - Extract command data from MMIO & save it for further use
> + * @ocxlpmem: the device metadata
> + * @offset: The base address of the command data structures (address of CREQO)
> + * @command_metadata: A pointer to the command metadata to populate
> + * Return: 0 on success, negative on failure
> + */
> +static int extract_command_metadata(struct ocxlpmem *ocxlpmem, u32 offset,
> +                                   struct command_metadata *command_metadata)

How about "struct ocxlpmem *ocp" throughout all these patches? The
full duplication of the type name as the local variable name makes
this look like non-idiomatic Linux code to me. It had not quite hit me
until I saw "struct command_metadata *command_metadata" that just
strikes me as too literal and the person that gets to maintain this
code later will appreciate a smaller amount of typing.

Also, is it really the case that the layout of the admin command
metadata needs to be programmatically determined at runtime? I would
expect it to be a static command definition in the spec.


> +{
> +       int rc;
> +       u64 tmp;
> +
> +       rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu, offset,
> +                                    OCXL_LITTLE_ENDIAN, &tmp);
> +       if (rc)
> +               return rc;
> +
> +       command_metadata->request_offset = tmp >> 32;
> +       command_metadata->response_offset = tmp & 0xFFFFFFFF;
> +
> +       rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu, offset + 8,
> +                                    OCXL_LITTLE_ENDIAN, &tmp);
> +       if (rc)
> +               return rc;
> +
> +       command_metadata->data_offset = tmp >> 32;
> +       command_metadata->data_size = tmp & 0xFFFFFFFF;
> +
> +       command_metadata->id = 0;
> +
> +       return 0;
> +}
> +
> +/**
> + * setup_command_metadata() - Set up the command metadata
> + * @ocxlpmem: the device metadata
> + */
> +static int setup_command_metadata(struct ocxlpmem *ocxlpmem)
> +{
> +       int rc;
> +
> +       mutex_init(&ocxlpmem->admin_command.lock);
> +
> +       rc = extract_command_metadata(ocxlpmem, GLOBAL_MMIO_ACMA_CREQO,
> +                                     &ocxlpmem->admin_command);
> +       if (rc)
> +               return rc;
> +
> +       return 0;
> +}
> +
>  /**
>   * allocate_minor() - Allocate a minor number to use for an OpenCAPI pmem device
>   * @ocxlpmem: the device metadata
> @@ -421,6 +473,14 @@ static int probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>
>         ocxlpmem->pdev = pci_dev_get(pdev);
>
> +       ocxlpmem->timeouts[ADMIN_COMMAND_ERRLOG] = 2000; // ms
> +       ocxlpmem->timeouts[ADMIN_COMMAND_HEARTBEAT] = 100; // ms
> +       ocxlpmem->timeouts[ADMIN_COMMAND_SMART] = 100; // ms
> +       ocxlpmem->timeouts[ADMIN_COMMAND_CONTROLLER_DUMP] = 1000; // ms
> +       ocxlpmem->timeouts[ADMIN_COMMAND_CONTROLLER_STATS] = 100; // ms
> +       ocxlpmem->timeouts[ADMIN_COMMAND_SHUTDOWN] = 1000; // ms
> +       ocxlpmem->timeouts[ADMIN_COMMAND_FW_UPDATE] = 16000; // ms
> +
>         pci_set_drvdata(pdev, ocxlpmem);
>
>         ocxlpmem->ocxl_fn = ocxl_function_open(pdev);
> @@ -467,6 +527,11 @@ static int probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>                 goto err;
>         }
>
> +       if (setup_command_metadata(ocxlpmem)) {
> +               dev_err(&pdev->dev, "Could not read command metadata\n");
> +               goto err;
> +       }
> +
>         elapsed = 0;
>         timeout = ocxlpmem->readiness_timeout +
>                   ocxlpmem->memory_available_timeout;
> diff --git a/drivers/nvdimm/ocxl/ocxlpmem.h b/drivers/nvdimm/ocxl/ocxlpmem.h
> index 3eadbe19f6d0..b72b3f909fc3 100644
> --- a/drivers/nvdimm/ocxl/ocxlpmem.h
> +++ b/drivers/nvdimm/ocxl/ocxlpmem.h
> @@ -7,6 +7,7 @@
>  #include <linux/mm.h>
>
>  #define LABEL_AREA_SIZE        BIT_ULL(PA_SECTION_SHIFT)
> +#define DEFAULT_TIMEOUT 100
>
>  #define GLOBAL_MMIO_CHI                0x000
>  #define GLOBAL_MMIO_CHIC       0x008
> @@ -57,6 +58,7 @@
>  #define GLOBAL_MMIO_HCI_CONTROLLER_DUMP_COLLECTED      BIT_ULL(5) // CDC
>  #define GLOBAL_MMIO_HCI_REQ_HEALTH_PERF                        BIT_ULL(6) // CHPD
>
> +// must be maintained with admin_command_names in ocxlpmem_internal.c
>  #define ADMIN_COMMAND_HEARTBEAT                0x00u
>  #define ADMIN_COMMAND_SHUTDOWN         0x01u
>  #define ADMIN_COMMAND_FW_UPDATE                0x02u
> @@ -68,6 +70,13 @@
>  #define ADMIN_COMMAND_CMD_CAPS         0x08u
>  #define ADMIN_COMMAND_MAX              0x08u
>
> +#define NS_COMMAND_SECURE_ERASE        0x20ull
> +
> +#define NS_RESPONSE_SECURE_ERASE_ACCESSIBLE_SUCCESS 0x20
> +#define NS_RESPONSE_SECURE_ERASE_ACCESSIBLE_ATTEMPTED 0x28
> +#define NS_RESPONSE_SECURE_ERASE_DEFECTIVE_SUCCESS 0x30
> +#define NS_RESPONSE_SECURE_ERASE_DEFECTIVE_ATTEMPTED 0x38
> +
>  #define STATUS_SUCCESS         0x00
>  #define STATUS_MEM_UNAVAILABLE 0x20
>  #define STATUS_BLOCKED_BG_TASK 0x21
> @@ -81,6 +90,16 @@
>  #define STATUS_FW_ARG_INVALID  STATUS_BAD_REQUEST_PARM
>  #define STATUS_FW_INVALID      STATUS_BAD_DATA_PARM
>
> +struct command_metadata {
> +       u32 request_offset;
> +       u32 response_offset;
> +       u32 data_offset;
> +       u32 data_size;
> +       struct mutex lock; /* locks access to this command */
> +       u16 id;
> +       u8 op_code;
> +};
> +
>  struct ocxlpmem {
>         struct device dev;
>         struct pci_dev *pdev;
> @@ -91,10 +110,11 @@ struct ocxlpmem {
>         struct ocxl_afu *ocxl_afu;
>         struct ocxl_context *ocxl_context;
>         void *metadata_addr;
> +       struct command_metadata admin_command;
>         struct resource pmem_res;
>         struct nd_region *nd_region;
>         char fw_version[8 + 1];
> -
> +       u32 timeouts[ADMIN_COMMAND_MAX + 1];
>         u32 max_controller_dump_size;
>         u16 scm_revision; // major/minor
>         u8 readiness_timeout;  /* The worst case time (in seconds) that the host
> @@ -123,3 +143,31 @@ struct ocxlpmem {
>   * Returns 0 on success, negative on error
>   */
>  int ocxlpmem_chi(const struct ocxlpmem *ocxlpmem, u64 *chi);
> +
> +/**
> + * admin_command_execute() - Execute an admin command and wait for completion
> + *
> + * Additional MMIO registers (dependent on the command) may
> + * need to be initialized
> + *
> + * @ocxlpmem: the device metadata
> + * @op_code: the code for the admin command
> + * Returns 0 on success, -EINVAL for a bad op code, -EBUSY on timeout
> + */
> +int admin_command_execute(struct ocxlpmem *ocxlpmem, u8 op_code);
> +
> +/**
> + * admin_response_handled() - Notify the controller that the admin response has been handled
> + * @ocxlpmem: the device metadata
> + * Returns 0 on success, negative on failure
> + */
> +int admin_response_handled(const struct ocxlpmem *ocxlpmem);
> +
> +/**
> + * warn_status() - Emit a kernel warning showing a command status.
> + * @ocxlpmem: the device metadata
> + * @message: A message to accompany the warning
> + * @status: The command status
> + */
> +void warn_status(const struct ocxlpmem *ocxlpmem, const char *message,
> +                u8 status);
> diff --git a/drivers/nvdimm/ocxl/ocxlpmem_internal.c b/drivers/nvdimm/ocxl/ocxlpmem_internal.c
> index 5578169b7515..7470a6ab3b08 100644
> --- a/drivers/nvdimm/ocxl/ocxlpmem_internal.c
> +++ b/drivers/nvdimm/ocxl/ocxlpmem_internal.c
> @@ -17,3 +17,264 @@ int ocxlpmem_chi(const struct ocxlpmem *ocxlpmem, u64 *chi)
>
>         return 0;
>  }
> +
> +#define COMMAND_REQUEST_SIZE (8 * sizeof(u64))
> +/**
> + * scm_command_request() - Set up a command request
> + * @cmd: The metadata for the type of command to be issued
> + * @op_code: the op code for the command
> + * @valid_bytes: the number of bytes in the header to preserve (these must be set before calling)
> + */
> +static int scm_command_request(const struct ocxlpmem *ocxlpmem,
> +                              struct command_metadata *cmd, u8 op_code,
> +                              u8 valid_bytes)

Ah, here's an abbreviation for command metadata.

> +{
> +       u64 val = op_code;
> +       int rc;
> +       u8 i;
> +
> +       cmd->op_code = op_code;
> +       cmd->id++;
> +
> +       val |= ((u64)cmd->id) << 16;
> +
> +       rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu, cmd->request_offset,
> +                                     OCXL_LITTLE_ENDIAN, val);
> +       if (rc)
> +               return rc;
> +
> +       for (i = valid_bytes; i < COMMAND_REQUEST_SIZE; i += sizeof(u64)) {
> +               rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu,
> +                                             cmd->request_offset + i,
> +                                             OCXL_LITTLE_ENDIAN, 0);
> +               if (rc)
> +                       return rc;
> +       }
> +
> +       return 0;
> +}
> +
> +/**
> + * admin_command_request() - Issue an admin command request
> + * @ocxlpmem: the device metadata
> + * @op_code: The op-code for the command
> + *
> + * Returns an identifier for the command, or negative on error
> + */
> +static int admin_command_request(struct ocxlpmem *ocxlpmem, u8 op_code)
> +{
> +       u8 valid_bytes = sizeof(u64);
> +
> +       switch (op_code) {
> +       case ADMIN_COMMAND_HEARTBEAT:
> +       case ADMIN_COMMAND_SHUTDOWN:
> +       case ADMIN_COMMAND_ERRLOG:
> +       case ADMIN_COMMAND_CMD_CAPS:
> +               valid_bytes += 0;
> +               break;
> +       case ADMIN_COMMAND_FW_UPDATE:
> +       case ADMIN_COMMAND_SMART:
> +       case ADMIN_COMMAND_CONTROLLER_STATS:
> +       case ADMIN_COMMAND_CONTROLLER_DUMP:
> +               valid_bytes += sizeof(u64);
> +               break;
> +       case ADMIN_COMMAND_FW_DEBUG:
> +               valid_bytes += 3 * sizeof(u64);
> +               break;
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       return scm_command_request(ocxlpmem, &ocxlpmem->admin_command, op_code,
> +                                  valid_bytes);

Why isn't this just:

    ocp_command_request(ocp, op_code, valid_bytes);

...because it's not clear to me why this switches from a generic name
like admin_command_request() and then switches to scm_. The
"command_metadata" is implied by the ocp context and the op_code,
right?


> +}
> +
> +static int command_response(const struct ocxlpmem *ocxlpmem,
> +                           const struct command_metadata *cmd)
> +{
> +       u64 val;
> +       u16 id;
> +       u8 status;
> +       int rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> +                                        cmd->response_offset,
> +                                        OCXL_LITTLE_ENDIAN, &val);
> +       if (rc)
> +               return rc;
> +
> +       status = val & 0xff;
> +       id = (val >> 16) & 0xffff;
> +
> +       if (id != cmd->id) {
> +               dev_err(&ocxlpmem->dev,
> +                       "Expected response for command %d, but received response for command %d instead.\n",
> +                       cmd->id, id);
> +               return -EBUSY;
> +       }
> +
> +       return status;
> +}
> +
> +/**
> + * admin_response() - Validate an admin response
> + * @ocxlpmem: the device metadata
> + * Returns the status code of the command, or negative on error
> + */
> +static int admin_response(const struct ocxlpmem *ocxlpmem)
> +{
> +       return command_response(ocxlpmem, &ocxlpmem->admin_command);
> +}
> +
> +/**
> + * admin_command_exec() - Notify the controller to start processing a pending admin command
> + * @ocxlpmem: the device metadata
> + * Returns 0 on success, negative on error
> + */
> +static int admin_command_exec(const struct ocxlpmem *ocxlpmem)
> +{
> +       return ocxl_global_mmio_set64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_HCI,
> +                                     OCXL_LITTLE_ENDIAN, GLOBAL_MMIO_HCI_ACRW);
> +}
> +
> +static bool admin_command_complete(const struct ocxlpmem *ocxlpmem)
> +{
> +       u64 val = 0;
> +
> +       int rc = ocxlpmem_chi(ocxlpmem, &val);
> +
> +       WARN_ON(rc);
> +
> +       return (val & GLOBAL_MMIO_CHI_ACRA) != 0;
> +}
> +
> +/**
> + * admin_command_complete_timeout() - Wait for an admin command to finish executing
> + * @ocxlpmem: the device metadata
> + * @command: the admin command to wait for completion (determines the timeout)
> + * Returns 0 on success, -EBUSY on timeout
> + */
> +static int admin_command_complete_timeout(const struct ocxlpmem *ocxlpmem,
> +                                         int command)
> +{
> +       unsigned long timeout = jiffies +
> +                               msecs_to_jiffies(ocxlpmem->timeouts[command]);
> +
> +       // 32 is the next power of 2 greater than the 20ms minimum for msleep
> +#define TIMEOUT_SLEEP_MILLIS 32
> +       do {
> +               if (admin_command_complete(ocxlpmem))
> +                       return 0;
> +               msleep(TIMEOUT_SLEEP_MILLIS);
> +       } while (time_before(jiffies, timeout));
> +
> +       if (admin_command_complete(ocxlpmem))
> +               return 0;
> +
> +       return -EBUSY;
> +}
> +
> +// Must be maintained with ADMIN_COMMAND_* in ocxlpmem.h
> +static const char * const admin_command_names[] = {
> +       "heartbeat",
> +       "shutdown",
> +       "firmware update",
> +       "firmware debug",
> +       "retrieve error log",
> +       "retrieve SMART data",
> +       "controller statistics",
> +       "controller dump",
> +       "command capabilities",
> +};
> +
> +/**
> + * admin_command_name() - get the name of an admin command
> + * @ocxlpmem: the device metadata
> + * @op_code: the code for the admin command
> + * Returns a string representing the name of the command
> + */
> +static const char *admin_command_name(u8 op_code)
> +{
> +       if (op_code > ADMIN_COMMAND_MAX)
> +               return "unknown command";
> +
> +       return admin_command_names[op_code];
> +}
> +
> +/**
> + * admin_command_execute() - Execute an admin command and wait for completion
> + *
> + * Additional MMIO registers (dependent on the command) may
> + * need to be initialized
> + *
> + * @ocxlpmem: the device metadata
> + * @op_code: the code for the admin command
> + * Returns 0 on success, -EINVAL for a bad op code, -EBUSY on timeout
> + */
> +int admin_command_execute(struct ocxlpmem *ocxlpmem, u8 op_code)
> +{
> +       int rc;
> +
> +       if (op_code > ADMIN_COMMAND_MAX)
> +               return -EINVAL;
> +
> +       rc = admin_command_request(ocxlpmem, op_code);
> +       if (rc)
> +               return rc;
> +
> +       rc = admin_command_exec(ocxlpmem);
> +       if (rc)
> +               return rc;
> +
> +       rc = admin_command_complete_timeout(ocxlpmem, op_code);
> +       if (rc < 0) {
> +               dev_warn(&ocxlpmem->dev, "%s timed out\n",
> +                        admin_command_name(op_code));
> +               return rc;
> +       }
> +
> +       return admin_response(ocxlpmem);
> +}
> +
> +int admin_response_handled(const struct ocxlpmem *ocxlpmem)
> +{
> +       // writing to the CHIC register clears the bit in CHI
> +       return ocxl_global_mmio_set64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_CHIC,
> +                                     OCXL_LITTLE_ENDIAN, GLOBAL_MMIO_CHI_ACRA);
> +}
> +
> +void warn_status(const struct ocxlpmem *ocxlpmem, const char *message,
> +                u8 status)
> +{
> +       const char *text = "Unknown";
> +
> +       switch (status) {
> +       case STATUS_SUCCESS:
> +               text = "Success";
> +               break;
> +
> +       case STATUS_MEM_UNAVAILABLE:
> +               text = "Persistent memory unavailable";
> +               break;
> +
> +       case STATUS_BAD_OPCODE:
> +               text = "Bad opcode";
> +               break;
> +
> +       case STATUS_BAD_REQUEST_PARM:
> +               text = "Bad request parameter";
> +               break;
> +
> +       case STATUS_BAD_DATA_PARM:
> +               text = "Bad data parameter";
> +               break;
> +
> +       case STATUS_DEBUG_BLOCKED:
> +               text = "Debug action blocked";
> +               break;
> +
> +       case STATUS_FAIL:
> +               text = "Failed";
> +               break;
> +       }
> +
> +       dev_warn(&ocxlpmem->dev, "%s: %s (%x)\n", message, text, status);
> +}
> --
> 2.24.1
>

^ permalink raw reply

* Re: [PATCH v4 06/25] ocxl: Tally up the LPC memory on a link & allow it to be mapped
From: Andrew Donnellan @ 2020-04-02  6:21 UTC (permalink / raw)
  To: Dan Williams, Alastair D'Silva
  Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
	Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
	Rob Herring, Dave Jiang, linux-nvdimm, Aneesh Kumar K . V,
	Krzysztof Kozlowski, Anju T Sudhakar, Mahesh Salgaonkar,
	Arnd Bergmann, Greg Kurz, Nicholas Piggin, Cédric Le Goater,
	Thomas Gleixner, Hari Bathini, Linux MM, Greg Kroah-Hartman,
	Linux Kernel Mailing List, Vishal Verma, Frederic Barrat,
	Paul Mackerras, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <CAPcyv4jyFQa5BDPCSQ6kmFY8CvWgbydePcn8B4M_Zyc1c7MGpg@mail.gmail.com>

On 1/4/20 7:48 pm, Dan Williams wrote:
> On Sun, Mar 29, 2020 at 10:53 PM Alastair D'Silva <alastair@d-silva.org> wrote:
>>
>> OpenCAPI LPC memory is allocated per link, but each link supports
>> multiple AFUs, and each AFU can have LPC memory assigned to it.
> 
> Is there an OpenCAPI primer to decode these objects and their
> associations that I can reference?

There isn't presently a primer that I think addresses these questions 
nicely (to my knowledge - Fred might have something he can link to?) - 
there are the specs published by the OpenCAPI Consortium at 
https://opencapi.org but they're really for hardware implementers.

We should probably expand what's currently documented in 
Documentation/userspace-api/accelerators/ocxl.rst generally, and this 
series should probably update that to include details on LPC.

To explain the specific objects here:

- A "link" is a point-to-point link between the host CPU, and a single 
OpenCAPI card. (We don't currently support cards making use of multiple 
links for increased bandwidth, though that is supported from a hardware 
point of view.)

- On POWER9, each link appears as a separate PCI domain, with a single 
bus, and the card appears as a single device.

- A device can have up to 8 functions, per PCI.

- An Attached Functional Unit (AFU) is the abstraction for a particular 
application function. Each PCI function defines the number of AFUs it 
has through a set of OpenCAPI-specific DVSECs, max 64 per function. The 
ocxl driver handles AFU discovery.

- On the host side, LPC memory is mapped by setting a single BAR for the 
whole link, but on the device side, LPC memory is requested on a per-AFU 
basis, through an AFU descriptor that is exposed through the 
aforementioned DVSECs. Hence the need to loop through the AFUs and get 
the total required LPC memory to work out the correct BAR value.

-- 
Andrew Donnellan              OzLabs, ADL Canberra
ajd@linux.ibm.com             IBM Australia Limited


^ permalink raw reply

* Re: [PATCH V2 3/5] selftests/powerpc: Add NX-GZIP engine compress testcase
From: Daniel Axtens @ 2020-04-02  6:17 UTC (permalink / raw)
  To: Raphael Moreira Zinsly, linuxppc-dev, linux-crypto
  Cc: Raphael Moreira Zinsly, haren, herbert, abali
In-Reply-To: <20200327181610.13762-4-rzinsly@linux.ibm.com>

Raphael Moreira Zinsly <rzinsly@linux.ibm.com> writes:

> Add a compression testcase for the powerpc NX-GZIP engine.
>
> Signed-off-by: Bulent Abali <abali@us.ibm.com>
> Signed-off-by: Raphael Moreira Zinsly <rzinsly@linux.ibm.com>
> ---
>  .../selftests/powerpc/nx-gzip/Makefile        |  21 +
>  .../selftests/powerpc/nx-gzip/gzfht_test.c    | 489 ++++++++++++++++++
>  .../selftests/powerpc/nx-gzip/gzip_vas.c      | 259 ++++++++++
>  3 files changed, 769 insertions(+)
>  create mode 100644 tools/testing/selftests/powerpc/nx-gzip/Makefile
>  create mode 100644 tools/testing/selftests/powerpc/nx-gzip/gzfht_test.c
>  create mode 100644 tools/testing/selftests/powerpc/nx-gzip/gzip_vas.c
>
> diff --git a/tools/testing/selftests/powerpc/nx-gzip/Makefile b/tools/testing/selftests/powerpc/nx-gzip/Makefile
> new file mode 100644
> index 000000000000..ab903f63bbbd
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/nx-gzip/Makefile
> @@ -0,0 +1,21 @@
> +CC = gcc
> +CFLAGS = -O3
> +INC = ./inc
> +SRC = gzfht_test.c
> +OBJ = $(SRC:.c=.o)
> +TESTS = gzfht_test
> +EXTRA_SOURCES = gzip_vas.c
> +
> +all:	$(TESTS)
> +
> +$(OBJ): %.o: %.c
> +	$(CC) $(CFLAGS) -I$(INC) -c $<
> +
> +$(TESTS): $(OBJ)
> +	$(CC) $(CFLAGS) -I$(INC) -o $@ $@.o $(EXTRA_SOURCES)
> +
> +run_tests: $(TESTS)
> +	./gzfht_test gzip_vas.c
> +
> +clean:
> +	rm -f $(TESTS) *.o *~ *.gz
> diff --git a/tools/testing/selftests/powerpc/nx-gzip/gzfht_test.c b/tools/testing/selftests/powerpc/nx-gzip/gzfht_test.c
> new file mode 100644
> index 000000000000..7a21c25f5611
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/nx-gzip/gzfht_test.c
> @@ -0,0 +1,489 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +/* P9 gzip sample code for demonstrating the P9 NX hardware interface.
> + * Not intended for productive uses or for performance or compression
> + * ratio measurements.  For simplicity of demonstration, this sample
> + * code compresses in to fixed Huffman blocks only (Deflate btype=1)
> + * and has very simple memory management.  Dynamic Huffman blocks
> + * (Deflate btype=2) are more involved as detailed in the user guide.
> + * Note also that /dev/crypto/gzip, VAS and skiboot support are
> + * required.
> + *
> + * Copyright 2020 IBM Corp.
> + *
> + * https://github.com/libnxz/power-gzip for zlib api and other utils
> + *
> + * Author: Bulent Abali <abali@us.ibm.com>
> + *
> + * Definitions of acronyms used here. See
> + * P9 NX Gzip Accelerator User's Manual for details:
> + * https://github.com/libnxz/power-gzip/blob/develop/doc/power_nx_gzip_um.pdf
> + *
> + * adler/crc: 32 bit checksums appended to stream tail
> + * ce:       completion extension
> + * cpb:      coprocessor parameter block (metadata)
> + * crb:      coprocessor request block (command)
> + * csb:      coprocessor status block (status)
> + * dht:      dynamic huffman table
> + * dde:      data descriptor element (address, length)
> + * ddl:      list of ddes
> + * dh/fh:    dynamic and fixed huffman types
> + * fc:       coprocessor function code
> + * histlen:  history/dictionary length
> + * history:  sliding window of up to 32KB of data
> + * lzcount:  Deflate LZ symbol counts
> + * rembytecnt: remaining byte count
> + * sfbt:     source final block type; last block's type during decomp
> + * spbc:     source processed byte count
> + * subc:     source unprocessed bit count
> + * tebc:     target ending bit count; valid bits in the last byte
> + * tpbc:     target processed byte count
> + * vas:      virtual accelerator switch; the user mode interface
> + */
> +
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <stdint.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <sys/time.h>
> +#include <sys/fcntl.h>
> +#include <sys/mman.h>
> +#include <endian.h>
> +#include <bits/endian.h>
> +#include <sys/ioctl.h>
> +#include <assert.h>
> +#include <errno.h>
> +#include <signal.h>
> +#include "nxu.h"
> +#include "nx.h"
> +
> +int nx_dbg;
> +FILE *nx_gzip_log;
> +void *nx_fault_storage_address;
> +
> +#define NX_MIN(X, Y) (((X) < (Y)) ? (X) : (Y))
> +#define FNAME_MAX 1024
> +#define FEXT ".nx.gz"
> +
> +/*
> + * LZ counts returned in the user supplied nx_gzip_crb_cpb_t structure.
> + */
> +static int compress_fht_sample(char *src, uint32_t srclen, char *dst,
> +				uint32_t dstlen, int with_count,
> +				struct nx_gzip_crb_cpb_t *cmdp, void *handle)
> +{
> +	int cc;
> +	uint32_t fc;
> +
> +	assert(!!cmdp);
> +
> +	put32(cmdp->crb, gzip_fc, 0);  /* clear */
> +	fc = (with_count) ? GZIP_FC_COMPRESS_RESUME_FHT_COUNT :
> +			    GZIP_FC_COMPRESS_RESUME_FHT;
> +	putnn(cmdp->crb, gzip_fc, fc);
> +	putnn(cmdp->cpb, in_histlen, 0); /* resuming with no history */
> +	memset((void *) &cmdp->crb.csb, 0, sizeof(cmdp->crb.csb));
> +
> +	/* Section 6.6 programming notes; spbc may be in two different
> +	 * places depending on FC.
> +	 */
> +	if (!with_count)
> +		put32(cmdp->cpb, out_spbc_comp, 0);
> +	else
> +		put32(cmdp->cpb, out_spbc_comp_with_count, 0);
> +
> +	/* Figure 6-3 6-4; CSB location */
> +	put64(cmdp->crb, csb_address, 0);
> +	put64(cmdp->crb, csb_address,
> +	      (uint64_t) &cmdp->crb.csb & csb_address_mask);
> +
> +	/* Source direct dde (scatter-gather list) */
> +	clear_dde(cmdp->crb.source_dde);
> +	putnn(cmdp->crb.source_dde, dde_count, 0);
> +	put32(cmdp->crb.source_dde, ddebc, srclen);
> +	put64(cmdp->crb.source_dde, ddead, (uint64_t) src);
> +
> +	/* Target direct dde (scatter-gather list) */
> +	clear_dde(cmdp->crb.target_dde);
> +	putnn(cmdp->crb.target_dde, dde_count, 0);
> +	put32(cmdp->crb.target_dde, ddebc, dstlen);
> +	put64(cmdp->crb.target_dde, ddead, (uint64_t) dst);
> +
> +	/* Submit the crb, the job descriptor, to the accelerator */
> +	nxu_run_job(cmdp, handle);
> +
> +	/* Poll for the csb.v bit; you should also consider sleeping
> +	 * or interrupts.
> +	 */
> +	do { ; } while (getnn(cmdp->crb.csb, csb_v) == 0);
> +
> +	/* CC Table 6-8 */
> +	cc = getnn(cmdp->crb.csb, csb_cc);
> +
> +	return cc;
> +}
> +
> +/*
> + * Prepares a blank no filename no timestamp gzip header and returns
> + * the number of bytes written to buf.
> + * Gzip specification at https://tools.ietf.org/html/rfc1952
> + */
> +int gzip_header_blank(char *buf)
> +{
> +	int i = 0;
> +
> +	buf[i++] = 0x1f; /* ID1 */
> +	buf[i++] = 0x8b; /* ID2 */
> +	buf[i++] = 0x08; /* CM  */
> +	buf[i++] = 0x00; /* FLG */
> +	buf[i++] = 0x00; /* MTIME */
> +	buf[i++] = 0x00; /* MTIME */
> +	buf[i++] = 0x00; /* MTIME */
> +	buf[i++] = 0x00; /* MTIME */
> +	buf[i++] = 0x04; /* XFL 4=fastest */
> +	buf[i++] = 0x03; /* OS UNIX */
> +
> +	return i;
> +}
> +
> +/* Caller must free the allocated buffer return nonzero on error. */
> +int read_alloc_input_file(char *fname, char **buf, size_t *bufsize)
> +{
> +	struct stat statbuf;
> +	FILE *fp;
> +	char *p;
> +	size_t num_bytes;
> +
> +	if (stat(fname, &statbuf)) {
> +		perror(fname);
> +		return(-1);
> +	}
> +	fp = fopen(fname, "r");
> +	if (fp == NULL) {
> +		perror(fname);
> +		return(-1);
> +	}
> +	assert(NULL != (p = (char *) malloc(statbuf.st_size)));
> +	num_bytes = fread(p, 1, statbuf.st_size, fp);
> +	if (ferror(fp) || (num_bytes != statbuf.st_size)) {
> +		perror(fname);
> +		return(-1);
> +	}
> +	*buf = p;
> +	*bufsize = num_bytes;
> +	return 0;
> +}
> +
> +/* Returns nonzero on error */
> +int write_output_file(char *fname, char *buf, size_t bufsize)
> +{
> +	FILE *fp;
> +	size_t num_bytes;
> +
> +	fp = fopen(fname, "w");
> +	if (fp == NULL) {
> +		perror(fname);
> +		return(-1);
> +	}
> +	num_bytes = fwrite(buf, 1, bufsize, fp);
> +	if (ferror(fp) || (num_bytes != bufsize)) {
> +		perror(fname);
> +		return(-1);
> +	}
> +	fclose(fp);
> +	return 0;
> +}
> +
> +/*
> + * Z_SYNC_FLUSH as described in zlib.h.
> + * Returns number of appended bytes
> + */
> +int append_sync_flush(char *buf, int tebc, int final)
> +{
> +	uint64_t flush;
> +	int shift = (tebc & 0x7);
> +
> +	if (tebc > 0) {
> +		/* Last byte is partially full */
> +		buf = buf - 1;
> +		*buf = *buf & (unsigned char) ((1<<tebc)-1);
> +	} else
> +		*buf = 0;
> +	flush = ((0x1ULL & final) << shift) | *buf;
> +	shift = shift + 3; /* BFINAL and BTYPE written */
> +	shift = (shift <= 8) ? 8 : 16;
> +	flush |= (0xFFFF0000ULL) << shift; /* Zero length block */
> +	shift = shift + 32;
> +	while (shift > 0) {
> +		*buf++ = (unsigned char) (flush & 0xffULL);
> +		flush = flush >> 8;
> +		shift = shift - 8;
> +	}
> +	return(((tebc > 5) || (tebc == 0)) ? 5 : 4);
> +}
> +
> +/*
> + * Fault in pages prior to NX job submission. wr=1 may be required to
> + * touch writeable pages.  System zero pages do not fault-in the page as
> + * intended.  Typically set wr=1 for NX target pages and set wr=0 for NX
> + * source pages.
> + */
> +static int nx_touch_pages(void *buf, long buf_len, long page_len, int wr)
> +{
> +	char *begin = buf;
> +	char *end = (char *) buf + buf_len - 1;
> +	char t;
> +
> +	assert(buf_len >= 0 && !!buf);
> +
> +	NXPRT(fprintf(stderr, "touch %p %p len 0x%lx wr=%d\n", buf,
> +			(buf + buf_len), buf_len, wr));
> +
> +	if (buf_len <= 0 || buf == NULL)
> +		return -1;
> +
> +	do {
> +		t = *begin;
> +		if (wr)
> +			*begin = t;
> +		begin = begin + page_len;
> +	} while (begin < end);
> +
> +	/* When buf_sz is small or buf tail is in another page */
> +	t = *end;
> +	if (wr)
> +		*end = t;
> +
> +	return 0;
> +}
> +
> +/*
> + * Final deflate block bit.  This call assumes the block
> + * beginning is byte aligned.
> + */
> +static void set_bfinal(void *buf, int bfinal)
> +{
> +	char *b = buf;
> +
> +	if (bfinal)
> +		*b = *b | (unsigned char) 0x01;
> +	else
> +		*b = *b & (unsigned char) 0xfe;
> +}
> +
> +int compress_file(int argc, char **argv, void *handle)
> +{
> +	char *inbuf, *outbuf, *srcbuf, *dstbuf;
> +	char outname[FNAME_MAX];
> +	uint32_t srclen, dstlen;
> +	uint32_t flushlen, chunk;
> +	size_t inlen, outlen, dsttotlen, srctotlen;
> +	uint32_t crc, spbc, tpbc, tebc;
> +	int lzcounts = 0;
> +	int cc;
> +	int num_hdr_bytes;
> +	struct nx_gzip_crb_cpb_t *cmdp;
> +	uint32_t pagelen = 65536;
> +	int fault_tries = 50;
> +
> +	cmdp = (void *)(uintptr_t)
> +		aligned_alloc(sizeof(struct nx_gzip_crb_cpb_t),
> +			      sizeof(struct nx_gzip_crb_cpb_t));
> +
> +	if (argc != 2) {
> +		fprintf(stderr, "usage: %s <fname>\n", argv[0]);
> +		exit(-1);
> +	}
> +	if (read_alloc_input_file(argv[1], &inbuf, &inlen))
> +		exit(-1);
> +	fprintf(stderr, "file %s read, %ld bytes\n", argv[1], inlen);
> +
> +	/* Generous output buffer for header/trailer */
> +	outlen = 2 * inlen + 1024;
> +
> +	assert(NULL != (outbuf = (char *)malloc(outlen)));
> +	nx_touch_pages(outbuf, outlen, pagelen, 1);
> +
> +	/* Compress piecemeal in smallish chunks */
> +	chunk = 1<<22;
> +
> +	/* Write the gzip header to the stream */
> +	num_hdr_bytes = gzip_header_blank(outbuf);
> +	dstbuf    = outbuf + num_hdr_bytes;
> +	outlen    = outlen - num_hdr_bytes;
> +	dsttotlen = num_hdr_bytes;
> +
> +	srcbuf    = inbuf;
> +	srctotlen = 0;
> +
> +	/* Init the CRB, the coprocessor request block */
> +	memset(&cmdp->crb, 0, sizeof(cmdp->crb));
> +
> +	/* Initial gzip crc32 */
> +	put32(cmdp->cpb, in_crc, 0);
> +
> +	fault_tries = 50;
> +
> +	while (inlen > 0) {
> +
> +		/* Submit chunk size source data per job */
> +		srclen = NX_MIN(chunk, inlen);
> +		/* Supply large target in case data expands */
> +		dstlen = NX_MIN(2*srclen, outlen);
> +
> +		/* Page faults are handled by the user code */
> +
> +		/* Fault-in pages; an improved code wouldn't touch so
> +		 * many pages but would try to estimate the
> +		 * compression ratio and adjust both the src and dst
> +		 * touch amounts.
> +		 */
> +		nx_touch_pages(cmdp, sizeof(struct nx_gzip_crb_cpb_t), pagelen,
> +				1);
> +		nx_touch_pages(srcbuf, srclen, pagelen, 0);
> +		nx_touch_pages(dstbuf, dstlen, pagelen, 1);
> +
> +		cc = compress_fht_sample(
> +			srcbuf, srclen,
> +			dstbuf, dstlen,
> +			lzcounts, cmdp, handle);
> +
> +		if (cc != ERR_NX_OK && cc != ERR_NX_TPBC_GT_SPBC &&
> +		    cc != ERR_NX_TRANSLATION) {
> +			fprintf(stderr, "nx error: cc= %d\n", cc);
> +			exit(-1);
> +		}
> +
> +		/* Page faults are handled by the user code */
> +		if (cc == ERR_NX_TRANSLATION) {
> +			NXPRT(fprintf(stderr, "page fault: cc= %d, ", cc));
> +			NXPRT(fprintf(stderr, "try= %d, fsa= %08llx\n",
> +				  fault_tries,
> +				  (unsigned long long) cmdp->crb.csb.fsaddr));
> +
> +			fault_tries--;
> +			if (fault_tries > 0) {
> +				continue;
> +			} else {
> +				fprintf(stderr, "error: cannot progress; ");
> +				fprintf(stderr, "too many faults\n");
> +				exit(-1);
> +			};
> +		}
> +
> +		fault_tries = 50; /* Reset for the next chunk */
> +
> +		inlen     = inlen - srclen;
> +		srcbuf    = srcbuf + srclen;
> +		srctotlen = srctotlen + srclen;
> +
> +		/* Two possible locations for spbc depending on the function
> +		 * code.
> +		 */
> +		spbc = (!lzcounts) ? get32(cmdp->cpb, out_spbc_comp) :
> +			get32(cmdp->cpb, out_spbc_comp_with_count);
> +		assert(spbc == srclen);
> +
> +		/* Target byte count */
> +		tpbc = get32(cmdp->crb.csb, tpbc);
> +		/* Target ending bit count */
> +		tebc = getnn(cmdp->cpb, out_tebc);
> +		NXPRT(fprintf(stderr, "compressed chunk %d " spbc));
> +		NXPRT(fprintf(stderr, "to %d bytes, tebc= %d\n", tpbc, tebc));
> +
> +		if (inlen > 0) { /* More chunks to go */
> +			set_bfinal(dstbuf, 0);
> +			dstbuf    = dstbuf + tpbc;
> +			dsttotlen = dsttotlen + tpbc;
> +			outlen    = outlen - tpbc;
> +			/* Round up to the next byte with a flush
> +			 * block; do not set the BFINAqL bit.
> +			 */
> +			flushlen  = append_sync_flush(dstbuf, tebc, 0);
> +			dsttotlen = dsttotlen + flushlen;
> +			outlen    = outlen - flushlen;
> +			dstbuf    = dstbuf + flushlen;
> +			NXPRT(fprintf(stderr, "added sync_flush %d bytes\n",
> +					flushlen));
> +		} else {  /* Done */
> +			/* Set the BFINAL bit of the last block per Deflate
> +			 * specification.
> +			 */
> +			set_bfinal(dstbuf, 1);
> +			dstbuf    = dstbuf + tpbc;
> +			dsttotlen = dsttotlen + tpbc;
> +			outlen    = outlen - tpbc;
> +		}
> +
> +		/* Resuming crc32 for the next chunk */
> +		crc = get32(cmdp->cpb, out_crc);
> +		put32(cmdp->cpb, in_crc, crc);
> +		crc = be32toh(crc);
> +	}
> +
> +	/* Append crc32 and ISIZE to the end */
> +	memcpy(dstbuf, &crc, 4);
> +	memcpy(dstbuf+4, &srctotlen, 4);
> +	dsttotlen = dsttotlen + 8;
> +	outlen    = outlen - 8;
> +
> +	assert(FNAME_MAX > (strlen(argv[1]) + strlen(FEXT)));
> +	strcpy(outname, argv[1]);
> +	strcat(outname, FEXT);
> +	if (write_output_file(outname, outbuf, dsttotlen)) {
> +		fprintf(stderr, "write error: %s\n", outname);
> +		exit(-1);
> +	}
> +
> +	fprintf(stderr, "compressed %ld to %ld bytes total, ", srctotlen,
> +		dsttotlen);
> +	fprintf(stderr, "crc32 checksum = %08x\n", crc);
> +
> +	if (inbuf != NULL)
> +		free(inbuf);
> +
> +	if (outbuf != NULL)
> +		free(outbuf);
> +
> +	return 0;
> +}
> +
> +void sigsegv_handler(int sig, siginfo_t *info, void *ctx)
> +{
> +	fprintf(stderr, "%d: Got signal %d si_code %d, si_addr %p\n", getpid(),
> +		sig, info->si_code, info->si_addr);
> +
> +	nx_fault_storage_address = info->si_addr;
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	int rc;
> +	struct sigaction act;
> +	void *handle;
> +
> +	nx_dbg = 0;
> +	nx_gzip_log = NULL;
> +	act.sa_handler = 0;
> +	act.sa_sigaction = sigsegv_handler;
> +	act.sa_flags = SA_SIGINFO;
> +	act.sa_restorer = 0;
> +	sigemptyset(&act.sa_mask);
> +	sigaction(SIGSEGV, &act, NULL);
> +
> +	handle = nx_function_begin(NX_FUNC_COMP_GZIP, 0);
> +	if (!handle) {
> +		fprintf(stderr, "Unable to init NX, errno %d\n", errno);
> +		exit(-1);
> +	}
> +
> +	rc = compress_file(argc, argv, handle);
> +
> +	nx_function_end(handle);
> +
> +	return rc;
> +}
> diff --git a/tools/testing/selftests/powerpc/nx-gzip/gzip_vas.c b/tools/testing/selftests/powerpc/nx-gzip/gzip_vas.c
> new file mode 100644
> index 000000000000..d28e1efb527b
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/nx-gzip/gzip_vas.c
> @@ -0,0 +1,259 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +/* Copyright 2020 IBM Corp.
> + *
> + * Author: Bulent Abali <abali@us.ibm.com>
> + *
> + */
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <stdint.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <sys/time.h>
> +#include <sys/fcntl.h>
> +#include <sys/mman.h>
> +#include <endian.h>
> +#include <bits/endian.h>
> +#include <sys/ioctl.h>
> +#include <assert.h>
> +#include <errno.h>
> +#include <signal.h>
> +#include "nx-gzip.h"
> +#include "nx.h"
> +#include "copy-paste.h"
> +#include "nxu.h"
> +#include "nx_dbg.h"
> +#include <sys/platform/ppc.h>
> +
> +#define barrier()
> +#define hwsync()    ({ asm volatile("hwsync" ::: "memory"); })

This doesn't compile on the clang version I tried as it doesn't
recognise 'hwsync'. Does
asm volatile("sync" ::: "memory");
do the same thing? That is recognised by clang, but I don't know if
dropping the hw prefix matters!

Regards,
Daniel

> +
> +#ifndef NX_NO_CPU_PRI
> +#define cpu_pri_default()  ({ asm volatile ("or 2, 2, 2"); })
> +#define cpu_pri_low()      ({ asm volatile ("or 31, 31, 31"); })
> +#else
> +#define cpu_pri_default()
> +#define cpu_pri_low()
> +#endif
> +
> +void *nx_fault_storage_address;
> +
> +struct nx_handle {
> +	int fd;
> +	int function;
> +	void *paste_addr;
> +};
> +
> +static int open_device_nodes(char *devname, int pri, struct nx_handle *handle)
> +{
> +	int rc, fd;
> +	void *addr;
> +	struct vas_gzip_setup_attr txattr;
> +
> +	fd = open(devname, O_RDWR);
> +	if (fd < 0) {
> +		fprintf(stderr, " open device name %s\n", devname);
> +		return -errno;
> +	}
> +
> +	memset(&txattr, 0, sizeof(txattr));
> +	txattr.version = 1;
> +	txattr.vas_id = pri;
> +	rc = ioctl(fd, VAS_GZIP_TX_WIN_OPEN, (unsigned long)&txattr);
> +	if (rc < 0) {
> +		fprintf(stderr, "ioctl() n %d, error %d\n", rc, errno);
> +		rc = -errno;
> +		goto out;
> +	}
> +
> +	addr = mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0ULL);
> +	if (addr == MAP_FAILED) {
> +		fprintf(stderr, "mmap() failed, errno %d\n", errno);
> +		rc = -errno;
> +		goto out;
> +	}
> +	handle->fd = fd;
> +	handle->paste_addr = (void *)((char *)addr + 0x400);
> +
> +	rc = 0;
> +out:
> +	close(fd);
> +	return rc;
> +}
> +
> +void *nx_function_begin(int function, int pri)
> +{
> +	int rc;
> +	char *devname = "/dev/crypto/nx-gzip";
> +	struct nx_handle *nxhandle;
> +
> +	if (function != NX_FUNC_COMP_GZIP) {
> +		errno = EINVAL;
> +		fprintf(stderr, " NX_FUNC_COMP_GZIP not found\n");
> +		return NULL;
> +	}
> +
> +
> +	nxhandle = malloc(sizeof(*nxhandle));
> +	if (!nxhandle) {
> +		errno = ENOMEM;
> +		fprintf(stderr, " No memory\n");
> +		return NULL;
> +	}
> +
> +	nxhandle->function = function;
> +	rc = open_device_nodes(devname, pri, nxhandle);
> +	if (rc < 0) {
> +		errno = -rc;
> +		fprintf(stderr, " open_device_nodes failed\n");
> +		return NULL;
> +	}
> +
> +	return nxhandle;
> +}
> +
> +int nx_function_end(void *handle)
> +{
> +	int rc = 0;
> +	struct nx_handle *nxhandle = handle;
> +
> +	rc = munmap(nxhandle->paste_addr - 0x400, 4096);
> +	if (rc < 0) {
> +		fprintf(stderr, "munmap() failed, errno %d\n", errno);
> +		return rc;
> +	}
> +	close(nxhandle->fd);
> +	free(nxhandle);
> +
> +	return rc;
> +}
> +
> +static int nx_wait_for_csb(struct nx_gzip_crb_cpb_t *cmdp)
> +{
> +	long poll = 0;
> +	uint64_t t;
> +
> +	/* Save power and let other threads use the h/w. top may show
> +	 * 100% but only because OS doesn't know we slowed the this
> +	 * h/w thread while polling. We're letting other threads have
> +	 * higher throughput on the core.
> +	 */
> +	cpu_pri_low();
> +
> +#define CSB_MAX_POLL 200000000UL
> +#define USLEEP_TH     300000UL
> +
> +	t = __ppc_get_timebase();
> +
> +	while (getnn(cmdp->crb.csb, csb_v) == 0) {
> +		++poll;
> +		hwsync();
> +
> +		cpu_pri_low();
> +
> +		/* usleep(0) takes around 29000 ticks ~60 us.
> +		 * 300000 is spinning for about 600 us then
> +		 * start sleeping.
> +		 */
> +		if ((__ppc_get_timebase() - t) > USLEEP_TH) {
> +			cpu_pri_default();
> +			usleep(1);
> +		}
> +
> +		if (poll > CSB_MAX_POLL)
> +			break;
> +
> +		/* Fault address from signal handler */
> +		if (nx_fault_storage_address) {
> +			cpu_pri_default();
> +			return -EAGAIN;
> +		}
> +
> +	}
> +
> +	cpu_pri_default();
> +
> +	/* hw has updated csb and output buffer */
> +	hwsync();
> +
> +	/* Check CSB flags. */
> +	if (getnn(cmdp->crb.csb, csb_v) == 0) {
> +		fprintf(stderr, "CSB still not valid after %d polls.\n",
> +			(int) poll);
> +		prt_err("CSB still not valid after %d polls, giving up.\n",
> +			(int) poll);
> +		return -ETIMEDOUT;
> +	}
> +
> +	return 0;
> +}
> +
> +int nxu_run_job(struct nx_gzip_crb_cpb_t *cmdp, void *handle)
> +{
> +	int i, ret, retries;
> +	struct nx_handle *nxhandle = handle;
> +
> +	assert(handle != NULL);
> +	i = 0;
> +	retries = 5000;
> +	while (i++ < retries) {
> +		hwsync();
> +		vas_copy(&cmdp->crb, 0);
> +		ret = vas_paste(nxhandle->paste_addr, 0);
> +		hwsync();
> +
> +		NXPRT(fprintf(stderr, "Paste attempt %d/%d returns 0x%x\n",
> +				i, retries, ret));
> +
> +		if ((ret == 2) || (ret == 3)) {
> +
> +			ret = nx_wait_for_csb(cmdp);
> +			if (!ret) {
> +				goto out;
> +			} else if (ret == -EAGAIN) {
> +				long x;
> +
> +				prt_err("Touching address %p, 0x%lx\n",
> +					 nx_fault_storage_address,
> +					 *(long *) nx_fault_storage_address);
> +				x = *(long *) nx_fault_storage_address;
> +				*(long *) nx_fault_storage_address = x;
> +				nx_fault_storage_address = 0;
> +				continue;
> +			} else {
> +				prt_err("wait_for_csb() returns %d\n", ret);
> +				break;
> +			}
> +		} else {
> +			if (i < 10) {
> +				/* spin for few ticks */
> +#define SPIN_TH 500UL
> +				uint64_t fail_spin;
> +
> +				fail_spin = __ppc_get_timebase();
> +				while ((__ppc_get_timebase() - fail_spin) <
> +					 SPIN_TH)
> +					;
> +			} else {
> +				/* sleep */
> +				unsigned int pr = 0;
> +
> +				if (pr++ % 100 == 0) {
> +					prt_err("Paste attempt %d/", i);
> +					prt_err("%d, failed pid= %d\n", retries,
> +						getpid());
> +				}
> +				usleep(1);
> +			}
> +			continue;
> +		}
> +	}
> +
> +out:
> +	cpu_pri_default();
> +
> +	return ret;
> +}
> -- 
> 2.21.0

^ permalink raw reply

* [powerpc:next-test] BUILD SUCCESS 824a2d10fcf429689cd20d7d36eeb24697466c9b
From: kbuild test robot @ 2020-04-02  5:01 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev

tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git  next-test
branch HEAD: 824a2d10fcf429689cd20d7d36eeb24697466c9b  powerpc/perf: split callchain.c by bitness

elapsed time: 1004m

configs tested: 192
configs skipped: 0

The following configs have been built successfully.
More configs may be tested in the coming days.

arm                              allmodconfig
arm                               allnoconfig
arm                              allyesconfig
arm64                            allmodconfig
arm64                             allnoconfig
arm64                            allyesconfig
arm                         at91_dt_defconfig
arm                           efm32_defconfig
arm                          exynos_defconfig
arm                        multi_v5_defconfig
arm                        multi_v7_defconfig
arm                        shmobile_defconfig
arm                           sunxi_defconfig
arm64                               defconfig
sparc                            allyesconfig
h8300                    h8300h-sim_defconfig
s390                              allnoconfig
sh                                allnoconfig
m68k                       m5475evb_defconfig
c6x                              allyesconfig
powerpc                       ppc64_defconfig
ia64                                defconfig
powerpc                             defconfig
sparc64                             defconfig
i386                              allnoconfig
i386                             alldefconfig
i386                             allyesconfig
i386                                defconfig
ia64                             alldefconfig
ia64                             allmodconfig
ia64                              allnoconfig
ia64                             allyesconfig
nios2                         3c120_defconfig
nios2                         10m50_defconfig
c6x                        evmc6678_defconfig
xtensa                          iss_defconfig
xtensa                       common_defconfig
openrisc                 simple_smp_defconfig
openrisc                    or1ksim_defconfig
alpha                               defconfig
csky                                defconfig
nds32                             allnoconfig
nds32                               defconfig
h8300                     edosk2674_defconfig
h8300                       h8s-sim_defconfig
m68k                             allmodconfig
m68k                          multi_defconfig
m68k                           sun3_defconfig
arc                              allyesconfig
arc                                 defconfig
microblaze                      mmu_defconfig
microblaze                    nommu_defconfig
powerpc                           allnoconfig
powerpc                          rhel-kconfig
mips                           32r2_defconfig
mips                         64r6el_defconfig
mips                             allmodconfig
mips                              allnoconfig
mips                             allyesconfig
mips                      fuloong2e_defconfig
mips                      malta_kvm_defconfig
parisc                            allnoconfig
parisc                           allyesconfig
parisc                generic-32bit_defconfig
parisc                generic-64bit_defconfig
x86_64               randconfig-a001-20200401
x86_64               randconfig-a002-20200401
x86_64               randconfig-a003-20200401
i386                 randconfig-a001-20200401
i386                 randconfig-a002-20200401
i386                 randconfig-a003-20200401
alpha                randconfig-a001-20200401
m68k                 randconfig-a001-20200401
mips                 randconfig-a001-20200401
nds32                randconfig-a001-20200401
parisc               randconfig-a001-20200401
riscv                randconfig-a001-20200401
mips                 randconfig-a001-20200402
nds32                randconfig-a001-20200402
m68k                 randconfig-a001-20200402
alpha                randconfig-a001-20200402
parisc               randconfig-a001-20200402
riscv                randconfig-a001-20200402
microblaze           randconfig-a001-20200331
h8300                randconfig-a001-20200331
nios2                randconfig-a001-20200331
c6x                  randconfig-a001-20200331
sparc64              randconfig-a001-20200331
c6x                  randconfig-a001-20200401
h8300                randconfig-a001-20200401
microblaze           randconfig-a001-20200401
nios2                randconfig-a001-20200401
sparc64              randconfig-a001-20200401
c6x                  randconfig-a001-20200402
h8300                randconfig-a001-20200402
microblaze           randconfig-a001-20200402
nios2                randconfig-a001-20200402
sparc64              randconfig-a001-20200402
s390                 randconfig-a001-20200401
xtensa               randconfig-a001-20200401
csky                 randconfig-a001-20200401
openrisc             randconfig-a001-20200401
sh                   randconfig-a001-20200401
x86_64               randconfig-b001-20200402
x86_64               randconfig-b002-20200402
x86_64               randconfig-b003-20200402
i386                 randconfig-b001-20200402
i386                 randconfig-b002-20200402
i386                 randconfig-b003-20200402
x86_64               randconfig-b001-20200401
x86_64               randconfig-b002-20200401
x86_64               randconfig-b003-20200401
i386                 randconfig-b001-20200401
i386                 randconfig-b002-20200401
i386                 randconfig-b003-20200401
x86_64               randconfig-c001-20200401
x86_64               randconfig-c002-20200401
x86_64               randconfig-c003-20200401
i386                 randconfig-c001-20200401
i386                 randconfig-c002-20200401
i386                 randconfig-c003-20200401
x86_64               randconfig-d001-20200401
x86_64               randconfig-d002-20200401
x86_64               randconfig-d003-20200401
i386                 randconfig-d001-20200401
i386                 randconfig-d002-20200401
i386                 randconfig-d003-20200401
x86_64               randconfig-e001-20200401
x86_64               randconfig-e002-20200401
x86_64               randconfig-e003-20200401
i386                 randconfig-e001-20200401
i386                 randconfig-e002-20200401
i386                 randconfig-e003-20200401
i386                 randconfig-f001-20200402
i386                 randconfig-f003-20200402
x86_64               randconfig-f003-20200402
x86_64               randconfig-f001-20200402
i386                 randconfig-f002-20200402
x86_64               randconfig-f002-20200402
x86_64               randconfig-f001-20200401
x86_64               randconfig-f002-20200401
x86_64               randconfig-f003-20200401
i386                 randconfig-f001-20200401
i386                 randconfig-f002-20200401
i386                 randconfig-f003-20200401
x86_64               randconfig-h001-20200401
x86_64               randconfig-h002-20200401
x86_64               randconfig-h003-20200401
i386                 randconfig-h001-20200401
i386                 randconfig-h002-20200401
i386                 randconfig-h003-20200401
arm64                randconfig-a001-20200401
sparc                randconfig-a001-20200401
ia64                 randconfig-a001-20200401
arc                  randconfig-a001-20200401
arm                  randconfig-a001-20200401
powerpc              randconfig-a001-20200401
arc                  randconfig-a001-20200402
arm                  randconfig-a001-20200402
arm64                randconfig-a001-20200402
ia64                 randconfig-a001-20200402
powerpc              randconfig-a001-20200402
sparc                randconfig-a001-20200402
riscv                            allmodconfig
riscv                             allnoconfig
riscv                            allyesconfig
riscv                               defconfig
riscv                    nommu_virt_defconfig
riscv                          rv32_defconfig
s390                             alldefconfig
s390                             allmodconfig
s390                             allyesconfig
s390                          debug_defconfig
s390                                defconfig
s390                       zfcpdump_defconfig
sh                               allmodconfig
sh                          rsk7269_defconfig
sh                  sh7785lcr_32bit_defconfig
sh                            titan_defconfig
sparc                               defconfig
sparc64                           allnoconfig
sparc64                          allyesconfig
sparc64                          allmodconfig
um                                  defconfig
um                             i386_defconfig
um                           x86_64_defconfig
x86_64                              fedora-25
x86_64                                  kexec
x86_64                                    lkp
x86_64                                   rhel
x86_64                         rhel-7.2-clear
x86_64                               rhel-7.6

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply

* [powerpc:merge] BUILD SUCCESS d0c12846a3a24cd6d68b608c866712bc7e471634
From: kbuild test robot @ 2020-04-02  4:56 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev

tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git  merge
branch HEAD: d0c12846a3a24cd6d68b608c866712bc7e471634  Automatic merge of branch 'next' into merge

elapsed time: 879m

configs tested: 176
configs skipped: 0

The following configs have been built successfully.
More configs may be tested in the coming days.

arm                              allmodconfig
arm                               allnoconfig
arm                              allyesconfig
arm64                            allmodconfig
arm64                             allnoconfig
arm64                            allyesconfig
arm                         at91_dt_defconfig
arm                           efm32_defconfig
arm                          exynos_defconfig
arm                        multi_v5_defconfig
arm                        multi_v7_defconfig
arm                        shmobile_defconfig
arm                           sunxi_defconfig
arm64                               defconfig
sparc                            allyesconfig
riscv                    nommu_virt_defconfig
ia64                                defconfig
powerpc                             defconfig
c6x                              allyesconfig
powerpc                       ppc64_defconfig
sparc64                             defconfig
s390                              allnoconfig
i386                              allnoconfig
i386                             alldefconfig
i386                             allyesconfig
i386                                defconfig
ia64                             allmodconfig
ia64                              allnoconfig
ia64                             allyesconfig
ia64                             alldefconfig
c6x                        evmc6678_defconfig
nios2                         10m50_defconfig
nios2                         3c120_defconfig
openrisc                    or1ksim_defconfig
openrisc                 simple_smp_defconfig
xtensa                       common_defconfig
xtensa                          iss_defconfig
alpha                               defconfig
csky                                defconfig
nds32                             allnoconfig
nds32                               defconfig
h8300                     edosk2674_defconfig
h8300                    h8300h-sim_defconfig
h8300                       h8s-sim_defconfig
m68k                             allmodconfig
m68k                       m5475evb_defconfig
m68k                          multi_defconfig
m68k                           sun3_defconfig
arc                              allyesconfig
arc                                 defconfig
microblaze                      mmu_defconfig
microblaze                    nommu_defconfig
powerpc                           allnoconfig
powerpc                          rhel-kconfig
mips                      fuloong2e_defconfig
mips                      malta_kvm_defconfig
mips                             allyesconfig
mips                         64r6el_defconfig
mips                              allnoconfig
mips                           32r2_defconfig
mips                             allmodconfig
parisc                            allnoconfig
parisc                           allyesconfig
parisc                generic-32bit_defconfig
parisc                generic-64bit_defconfig
x86_64               randconfig-a001-20200401
x86_64               randconfig-a002-20200401
x86_64               randconfig-a003-20200401
i386                 randconfig-a001-20200401
i386                 randconfig-a002-20200401
i386                 randconfig-a003-20200401
alpha                randconfig-a001-20200401
m68k                 randconfig-a001-20200401
mips                 randconfig-a001-20200401
nds32                randconfig-a001-20200401
parisc               randconfig-a001-20200401
riscv                randconfig-a001-20200401
mips                 randconfig-a001-20200402
c6x                  randconfig-a001-20200401
h8300                randconfig-a001-20200401
microblaze           randconfig-a001-20200401
nios2                randconfig-a001-20200401
sparc64              randconfig-a001-20200401
c6x                  randconfig-a001-20200402
h8300                randconfig-a001-20200402
microblaze           randconfig-a001-20200402
nios2                randconfig-a001-20200402
sparc64              randconfig-a001-20200402
csky                 randconfig-a001-20200401
openrisc             randconfig-a001-20200401
s390                 randconfig-a001-20200401
sh                   randconfig-a001-20200401
xtensa               randconfig-a001-20200401
x86_64               randconfig-b001-20200402
x86_64               randconfig-b002-20200402
x86_64               randconfig-b003-20200402
i386                 randconfig-b001-20200402
i386                 randconfig-b002-20200402
i386                 randconfig-b003-20200402
x86_64               randconfig-b001-20200401
x86_64               randconfig-b002-20200401
x86_64               randconfig-b003-20200401
i386                 randconfig-b001-20200401
i386                 randconfig-b002-20200401
i386                 randconfig-b003-20200401
x86_64               randconfig-c001-20200401
x86_64               randconfig-c002-20200401
x86_64               randconfig-c003-20200401
i386                 randconfig-c001-20200401
i386                 randconfig-c002-20200401
i386                 randconfig-c003-20200401
x86_64               randconfig-d001-20200401
x86_64               randconfig-d002-20200401
x86_64               randconfig-d003-20200401
i386                 randconfig-d001-20200401
i386                 randconfig-d002-20200401
i386                 randconfig-d003-20200401
x86_64               randconfig-e001-20200401
x86_64               randconfig-e002-20200401
x86_64               randconfig-e003-20200401
i386                 randconfig-e001-20200401
i386                 randconfig-e002-20200401
i386                 randconfig-e003-20200401
i386                 randconfig-f001-20200402
i386                 randconfig-f003-20200402
x86_64               randconfig-f003-20200402
x86_64               randconfig-f001-20200402
i386                 randconfig-f002-20200402
x86_64               randconfig-f002-20200402
x86_64               randconfig-h001-20200401
x86_64               randconfig-h002-20200401
x86_64               randconfig-h003-20200401
i386                 randconfig-h001-20200401
i386                 randconfig-h002-20200401
i386                 randconfig-h003-20200401
arc                  randconfig-a001-20200402
arm                  randconfig-a001-20200402
arm64                randconfig-a001-20200402
ia64                 randconfig-a001-20200402
powerpc              randconfig-a001-20200402
sparc                randconfig-a001-20200402
arm64                randconfig-a001-20200401
sparc                randconfig-a001-20200401
ia64                 randconfig-a001-20200401
arc                  randconfig-a001-20200401
powerpc              randconfig-a001-20200401
arm                  randconfig-a001-20200401
riscv                            allyesconfig
riscv                             allnoconfig
riscv                               defconfig
riscv                          rv32_defconfig
riscv                            allmodconfig
s390                             alldefconfig
s390                             allmodconfig
s390                             allyesconfig
s390                          debug_defconfig
s390                                defconfig
s390                       zfcpdump_defconfig
sh                               allmodconfig
sh                                allnoconfig
sh                          rsk7269_defconfig
sh                  sh7785lcr_32bit_defconfig
sh                            titan_defconfig
sparc                               defconfig
sparc64                          allmodconfig
sparc64                           allnoconfig
sparc64                          allyesconfig
um                                  defconfig
um                             i386_defconfig
um                           x86_64_defconfig
x86_64                              fedora-25
x86_64                                  kexec
x86_64                                    lkp
x86_64                                   rhel
x86_64                         rhel-7.2-clear
x86_64                               rhel-7.6

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply

* Re: [PATCH v2 1/1] ppc/crash: Skip spinlocks during crash
From: kbuild test robot @ 2020-04-02  4:49 UTC (permalink / raw)
  To: Leonardo Bras
  Cc: kbuild-all, Greg Kroah-Hartman, Peter Zijlstra, linuxppc-dev,
	linux-kernel, Alexios Zavras, Ingo Molnar, Paul Mackerras,
	Leonardo Bras, Thomas Gleixner, Will Deacon
In-Reply-To: <20200326232542.503157-1-leonardo@linux.ibm.com>

[-- Attachment #1: Type: text/plain, Size: 3154 bytes --]

Hi Leonardo,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/locking/core]
[also build test ERROR on powerpc/next paulus-powerpc/kvm-ppc-next v5.6 next-20200401]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Leonardo-Bras/ppc-crash-Skip-spinlocks-during-crash/20200327-105958
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 8bf6c677ddb9c922423ea3bf494fe7c508bfbb8c
config: powerpc-randconfig-a001-20200401 (attached as .config)
compiler: powerpc-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=9.3.0 make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   powerpc-linux-ld: arch/powerpc/kernel/traps.o: in function `arch_spin_lock':
>> arch/powerpc/include/asm/spinlock.h:147: undefined reference to `crash_skip_spinlock'
>> powerpc-linux-ld: arch/powerpc/include/asm/spinlock.h:147: undefined reference to `crash_skip_spinlock'
   powerpc-linux-ld: arch/powerpc/kernel/rtas.o: in function `arch_spin_lock':
>> arch/powerpc/include/asm/spinlock.h:147: undefined reference to `crash_skip_spinlock'
>> powerpc-linux-ld: arch/powerpc/include/asm/spinlock.h:147: undefined reference to `crash_skip_spinlock'
   powerpc-linux-ld: kernel/locking/lockdep.o: in function `arch_spin_lock':
>> arch/powerpc/include/asm/spinlock.h:147: undefined reference to `crash_skip_spinlock'
   powerpc-linux-ld: kernel/locking/lockdep.o:arch/powerpc/include/asm/spinlock.h:147: more undefined references to `crash_skip_spinlock' follow
>> pahole: .tmp_vmlinux.btf: No such file or directory
   powerpc-linux-objdump: '.tmp_vmlinux.btf': No such file
   powerpc-linux-objdump: '.tmp_vmlinux.btf': No such file
   powerpc-linux-objcopy: '.tmp_vmlinux.btf': No such file
   powerpc-linux-objcopy: --change-section-vma .BTF=0x0000000000000000 never used
   powerpc-linux-objcopy: --change-section-lma .BTF=0x0000000000000000 never used
   powerpc-linux-objcopy: '.btf.vmlinux.bin': No such file
   Failed to generate BTF for vmlinux
   Try to disable CONFIG_DEBUG_INFO_BTF

vim +147 arch/powerpc/include/asm/spinlock.h

   140	
   141	static inline void arch_spin_lock(arch_spinlock_t *lock)
   142	{
   143		while (1) {
   144			if (likely(__arch_spin_trylock(lock) == 0))
   145				break;
   146			do {
 > 147				if (unlikely(crash_skip_spinlock))
   148					return;
   149				HMT_low();
   150				if (is_shared_processor())
   151					splpar_spin_yield(lock);
   152			} while (unlikely(lock->slock != 0));
   153			HMT_medium();
   154		}
   155	}
   156	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28254 bytes --]

^ permalink raw reply

* RE: [PATCH v4 03/25] powerpc/powernv: Map & release OpenCAPI LPC memory
From: Alastair D'Silva @ 2020-04-02  4:36 UTC (permalink / raw)
  To: 'Dan Williams'
  Cc: 'Madhavan Srinivasan', 'Alexey Kardashevskiy',
	'Masahiro Yamada', 'Oliver O'Halloran',
	'Mauro Carvalho Chehab', 'Ira Weiny',
	'Rob Herring', 'Dave Jiang',
	'linux-nvdimm', 'Aneesh Kumar K . V',
	'Krzysztof Kozlowski', 'Anju T Sudhakar',
	'Mahesh Salgaonkar', 'Andrew Donnellan',
	'Arnd Bergmann', 'Greg Kurz',
	'Nicholas Piggin', 'Cédric Le Goater',
	'Thomas Gleixner', 'Hari Bathini',
	'Linux MM', 'Greg Kroah-Hartman',
	'Linux Kernel Mailing List', 'Vishal Verma',
	'Frederic Barrat', 'Paul Mackerras',
	'Andrew Morton', 'linuxppc-dev',
	'David S. Miller'
In-Reply-To: <CAPcyv4iGEHJpZctEm+Do1-kOZBUDeKKcREr=BqcK4kCvLWhAQQ@mail.gmail.com>

> -----Original Message-----
> From: Dan Williams <dan.j.williams@intel.com>
> Sent: Wednesday, 1 April 2020 7:49 PM
> To: Alastair D'Silva <alastair@d-silva.org>
> Cc: Aneesh Kumar K . V <aneesh.kumar@linux.ibm.com>; Oliver O'Halloran
> <oohall@gmail.com>; Benjamin Herrenschmidt
> <benh@kernel.crashing.org>; Paul Mackerras <paulus@samba.org>; Michael
> Ellerman <mpe@ellerman.id.au>; Frederic Barrat <fbarrat@linux.ibm.com>;
> Andrew Donnellan <ajd@linux.ibm.com>; Arnd Bergmann
> <arnd@arndb.de>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>;
> Vishal Verma <vishal.l.verma@intel.com>; Dave Jiang
> <dave.jiang@intel.com>; Ira Weiny <ira.weiny@intel.com>; Andrew Morton
> <akpm@linux-foundation.org>; Mauro Carvalho Chehab
> <mchehab+samsung@kernel.org>; David S. Miller <davem@davemloft.net>;
> Rob Herring <robh@kernel.org>; Anton Blanchard <anton@ozlabs.org>;
> Krzysztof Kozlowski <krzk@kernel.org>; Mahesh Salgaonkar
> <mahesh@linux.vnet.ibm.com>; Madhavan Srinivasan
> <maddy@linux.vnet.ibm.com>; Cédric Le Goater <clg@kaod.org>; Anju T
> Sudhakar <anju@linux.vnet.ibm.com>; Hari Bathini
> <hbathini@linux.ibm.com>; Thomas Gleixner <tglx@linutronix.de>; Greg
> Kurz <groug@kaod.org>; Nicholas Piggin <npiggin@gmail.com>; Masahiro
> Yamada <yamada.masahiro@socionext.com>; Alexey Kardashevskiy
> <aik@ozlabs.ru>; Linux Kernel Mailing List <linux-kernel@vger.kernel.org>;
> linuxppc-dev <linuxppc-dev@lists.ozlabs.org>; linux-nvdimm <linux-
> nvdimm@lists.01.org>; Linux MM <linux-mm@kvack.org>
> Subject: Re: [PATCH v4 03/25] powerpc/powernv: Map & release OpenCAPI
> LPC memory
> 
> On Sun, Mar 29, 2020 at 10:23 PM Alastair D'Silva <alastair@d-silva.org>
> wrote:
> >
> > This patch adds OPAL calls to powernv so that the OpenCAPI driver can
> > map & release LPC (Lowest Point of Coherency)  memory.
> >
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
> > ---
> >  arch/powerpc/include/asm/pnv-ocxl.h   |  2 ++
> >  arch/powerpc/platforms/powernv/ocxl.c | 43
> > +++++++++++++++++++++++++++
> >  2 files changed, 45 insertions(+)
> >
> > diff --git a/arch/powerpc/include/asm/pnv-ocxl.h
> > b/arch/powerpc/include/asm/pnv-ocxl.h
> > index 7de82647e761..560a19bb71b7 100644
> > --- a/arch/powerpc/include/asm/pnv-ocxl.h
> > +++ b/arch/powerpc/include/asm/pnv-ocxl.h
> > @@ -32,5 +32,7 @@ extern int
> pnv_ocxl_spa_remove_pe_from_cache(void
> > *platform_data, int pe_handle)
> >
> >  extern int pnv_ocxl_alloc_xive_irq(u32 *irq, u64 *trigger_addr);
> > extern void pnv_ocxl_free_xive_irq(u32 irq);
> > +u64 pnv_ocxl_platform_lpc_setup(struct pci_dev *pdev, u64 size); void
> > +pnv_ocxl_platform_lpc_release(struct pci_dev *pdev);
> >
> >  #endif /* _ASM_PNV_OCXL_H */
> > diff --git a/arch/powerpc/platforms/powernv/ocxl.c
> > b/arch/powerpc/platforms/powernv/ocxl.c
> > index 8c65aacda9c8..f13119a7c026 100644
> > --- a/arch/powerpc/platforms/powernv/ocxl.c
> > +++ b/arch/powerpc/platforms/powernv/ocxl.c
> > @@ -475,6 +475,49 @@ void pnv_ocxl_spa_release(void *platform_data)
> }
> > EXPORT_SYMBOL_GPL(pnv_ocxl_spa_release);
> >
> > +u64 pnv_ocxl_platform_lpc_setup(struct pci_dev *pdev, u64 size) {
> > +       struct pci_controller *hose = pci_bus_to_host(pdev->bus);
> > +       struct pnv_phb *phb = hose->private_data;
> 
> Is calling the local variable 'hose' instead of 'host' on purpose?
> 

Yes, this follows the convention used in other functions in this file.

> > +       u32 bdfn = pci_dev_id(pdev);
> > +       __be64 base_addr_be64;
> > +       u64 base_addr;
> > +       int rc;
> > +
> > +       rc = opal_npu_mem_alloc(phb->opal_id, bdfn, size,
> &base_addr_be64);
> > +       if (rc) {
> > +               dev_warn(&pdev->dev,
> > +                        "OPAL could not allocate LPC memory, rc=%d\n", rc);
> > +               return 0;
> > +       }
> > +
> > +       base_addr = be64_to_cpu(base_addr_be64);
> > +
> > +#ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
> 
> With the proposed cleanup in patch2 the ifdef can be elided here.

Ok
> 
> > +       rc = check_hotplug_memory_addressable(base_addr >> PAGE_SHIFT,
> > +                                             size >> PAGE_SHIFT);
> > +       if (rc)
> > +               return 0;
> 
> Is this an error worth logging if someone is wondering why their device is not
> showing up?
> 

Yes, I'll add a message.

> 
> > +#endif
> > +
> > +       return base_addr;
> > +}
> > +EXPORT_SYMBOL_GPL(pnv_ocxl_platform_lpc_setup);
> > +
> > +void pnv_ocxl_platform_lpc_release(struct pci_dev *pdev) {
> > +       struct pci_controller *hose = pci_bus_to_host(pdev->bus);
> > +       struct pnv_phb *phb = hose->private_data;
> > +       u32 bdfn = pci_dev_id(pdev);
> > +       int rc;
> > +
> > +       rc = opal_npu_mem_release(phb->opal_id, bdfn);
> > +       if (rc)
> > +               dev_warn(&pdev->dev,
> > +                        "OPAL reported rc=%d when releasing LPC
> > +memory\n", rc); }
> EXPORT_SYMBOL_GPL(pnv_ocxl_platform_lpc_release);
> > +
> >  int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int
> > pe_handle)  {
> >         struct spa_data *data = (struct spa_data *) platform_data;
> > --
> > 2.24.1
> >
> 
> 
> --
> This email has been checked for viruses by AVG.
> https://www.avg.com



^ permalink raw reply

* RE: [PATCH v4 02/25] mm/memory_hotplug: Allow check_hotplug_memory_addressable to be called from drivers
From: Alastair D'Silva @ 2020-04-02  4:33 UTC (permalink / raw)
  To: 'Dan Williams'
  Cc: 'Madhavan Srinivasan', 'Alexey Kardashevskiy',
	'Masahiro Yamada', 'Oliver O'Halloran',
	'Mauro Carvalho Chehab', 'Ira Weiny',
	'Rob Herring', 'Dave Jiang',
	'linux-nvdimm', 'Aneesh Kumar K . V',
	'Krzysztof Kozlowski', 'Anju T Sudhakar',
	'Mahesh Salgaonkar', 'Andrew Donnellan',
	'Arnd Bergmann', 'Greg Kurz',
	'Nicholas Piggin', 'Cédric Le Goater',
	'Thomas Gleixner', 'Hari Bathini',
	'Linux MM', 'Greg Kroah-Hartman',
	'Linux Kernel Mailing List', 'Vishal Verma',
	'Frederic Barrat', 'Paul Mackerras',
	'Andrew Morton', 'linuxppc-dev',
	'David S. Miller'
In-Reply-To: <CAPcyv4h9uDxHDb0iN+zwhPB=By8Ps8cwTyipf6b64v+ruzhchg@mail.gmail.com>

> -----Original Message-----
> From: Dan Williams <dan.j.williams@intel.com>
> Sent: Wednesday, 1 April 2020 7:48 PM
> To: Alastair D'Silva <alastair@d-silva.org>
> Cc: Aneesh Kumar K . V <aneesh.kumar@linux.ibm.com>; Oliver O'Halloran
> <oohall@gmail.com>; Benjamin Herrenschmidt
> <benh@kernel.crashing.org>; Paul Mackerras <paulus@samba.org>; Michael
> Ellerman <mpe@ellerman.id.au>; Frederic Barrat <fbarrat@linux.ibm.com>;
> Andrew Donnellan <ajd@linux.ibm.com>; Arnd Bergmann
> <arnd@arndb.de>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>;
> Vishal Verma <vishal.l.verma@intel.com>; Dave Jiang
> <dave.jiang@intel.com>; Ira Weiny <ira.weiny@intel.com>; Andrew Morton
> <akpm@linux-foundation.org>; Mauro Carvalho Chehab
> <mchehab+samsung@kernel.org>; David S. Miller <davem@davemloft.net>;
> Rob Herring <robh@kernel.org>; Anton Blanchard <anton@ozlabs.org>;
> Krzysztof Kozlowski <krzk@kernel.org>; Mahesh Salgaonkar
> <mahesh@linux.vnet.ibm.com>; Madhavan Srinivasan
> <maddy@linux.vnet.ibm.com>; Cédric Le Goater <clg@kaod.org>; Anju T
> Sudhakar <anju@linux.vnet.ibm.com>; Hari Bathini
> <hbathini@linux.ibm.com>; Thomas Gleixner <tglx@linutronix.de>; Greg
> Kurz <groug@kaod.org>; Nicholas Piggin <npiggin@gmail.com>; Masahiro
> Yamada <yamada.masahiro@socionext.com>; Alexey Kardashevskiy
> <aik@ozlabs.ru>; Linux Kernel Mailing List <linux-kernel@vger.kernel.org>;
> linuxppc-dev <linuxppc-dev@lists.ozlabs.org>; linux-nvdimm <linux-
> nvdimm@lists.01.org>; Linux MM <linux-mm@kvack.org>
> Subject: Re: [PATCH v4 02/25] mm/memory_hotplug: Allow
> check_hotplug_memory_addressable to be called from drivers
> 
> On Sun, Mar 29, 2020 at 10:23 PM Alastair D'Silva <alastair@d-silva.org>
> wrote:
> >
> > When setting up OpenCAPI connected persistent memory, the range check
> > may not be performed until quite late (or perhaps not at all, if the
> > user does not establish a DAX device).
> >
> > This patch makes the range check callable so we can perform the check
> > while probing the OpenCAPI Persistent Memory device.
> >
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
> > ---
> >  include/linux/memory_hotplug.h | 5 +++++
> >  mm/memory_hotplug.c            | 4 ++--
> >  2 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/memory_hotplug.h
> > b/include/linux/memory_hotplug.h index f4d59155f3d4..9a19ae0d7e31
> > 100644
> > --- a/include/linux/memory_hotplug.h
> > +++ b/include/linux/memory_hotplug.h
> > @@ -337,6 +337,11 @@ static inline void __remove_memory(int nid, u64
> > start, u64 size) {}  extern void set_zone_contiguous(struct zone
> > *zone);  extern void clear_zone_contiguous(struct zone *zone);
> >
> > +#ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
> > +int check_hotplug_memory_addressable(unsigned long pfn,
> > +                                    unsigned long nr_pages); #endif
> > +/* CONFIG_MEMORY_HOTPLUG_SPARSE */
> 
> Let's move this to include/linux/memory.h with the other
> CONFIG_MEMORY_HOTPLUG_SPARSE declarations, and add a dummy
> implementation for the CONFIG_MEMORY_HOTPLUG_SPARSE=n case.
> 
> Also, this patch can be squashed with the next one, no need for it to be
> stand alone.
> 

Ok

> 
> > +
> >  extern void __ref free_area_init_core_hotplug(int nid);  extern int
> > __add_memory(int nid, u64 start, u64 size);  extern int add_memory(int
> > nid, u64 start, u64 size); diff --git a/mm/memory_hotplug.c
> > b/mm/memory_hotplug.c index 0a54ffac8c68..14945f033594 100644
> > --- a/mm/memory_hotplug.c
> > +++ b/mm/memory_hotplug.c
> > @@ -276,8 +276,8 @@ static int check_pfn_span(unsigned long pfn,
> unsigned long nr_pages,
> >         return 0;
> >  }
> >
> > -static int check_hotplug_memory_addressable(unsigned long pfn,
> > -                                           unsigned long nr_pages)
> > +int check_hotplug_memory_addressable(unsigned long pfn,
> > +                                    unsigned long nr_pages)
> >  {
> >         const u64 max_addr = PFN_PHYS(pfn + nr_pages) - 1;
> >
> > --
> > 2.24.1
> >

-- 
Alastair D'Silva           mob: 0423 762 819
skype: alastair_dsilva     msn: alastair@d-silva.org
blog: http://alastair.d-silva.org    Twitter: @EvilDeece
 



^ permalink raw reply

* Re: [RFC PATCH 3/4] powerpc ppc-opcode: move ppc instuction encoding from test_emulate_step
From: Michael Ellerman @ 2020-04-02  4:25 UTC (permalink / raw)
  To: Naveen N. Rao, Balamuruhan S
  Cc: jniethe5, linuxppc-dev, sandipan, paulus, ravi.bangoria
In-Reply-To: <1585759165.2r0pc78n07.naveen@linux.ibm.com>

"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
> Balamuruhan S wrote:
>> Few ppc instructions are encoded in test_emulate_step.c, consolidate them to
>> ppc-opcode.h, fix redefintion errors in bpf_jit caused due to this consolidation.
>> Reuse the macros from ppc-opcode.h
...
>> diff --git a/arch/powerpc/net/bpf_jit32.h b/arch/powerpc/net/bpf_jit32.h
>> index 4ec2a9f14f84..8a9f16a7262e 100644
>> --- a/arch/powerpc/net/bpf_jit32.h
>> +++ b/arch/powerpc/net/bpf_jit32.h
>> @@ -76,13 +76,13 @@ DECLARE_LOAD_FUNC(sk_load_byte_msh);
>>  		else {	PPC_ADDIS(r, base, IMM_HA(i));			      \
>>  			PPC_LBZ(r, r, IMM_L(i)); } } while(0)
>> 
>> -#define PPC_LD_OFFS(r, base, i) do { if ((i) < 32768) PPC_LD(r, base, i);     \
>> +#define _OFFS(r, base, i) do { if ((i) < 32768) EMIT(PPC_ENCODE_LD(r, base, i));     \
> 	   ^^^^^
> Should be PPC_LD_OFFS. For the next version, please also build ppc32 and 
> booke codebase to confirm that your changes in those areas are fine.
>
> PPC_ENCODE_* also looks quite verbose, so perhaps PPC_ENC_* might be 
> better. Otherwise, this patchset looks good to me and should help reuse 
> some of those macros, especially from the eBPF codebase.
>
> Michael,
> Can you let us know if this looks ok to you? Based on your feedback, we 
> will also update the eBPF codebase.

I didn't really like the first patch which does the mass renaming. It
creates a huge amount of churn.

I think I'd be happier if this series just did what it needs, and then
maybe at the end there's a patch to update all the existing names, which
I may or may not take.

As far as the naming, currently we have:

PPC_INST_FOO - just the opcode

PPC_FOO(x) - macro to encode the opcode with x and (usually) also emit a
            .long and stringify.

And you need an in-between that gives you the full instruction but
without the .long and stringify, right?

So how about PPC_RAW_FOO() for just the numeric value, without the .long
and stringify.

We also seem to have a lot of PPC_INST_FOO's that are only ever used in
the PPC_INST macro. I'm inclined to fold those into the PPC_INST macro,
to avoid people accidentally using the PPC_INST version when they don't
mean to. But that's a separate issue.

cheers

^ permalink raw reply

* Re: [PATCH v4 00/25] Add support for OpenCAPI Persistent Memory devices
From: Oliver O'Halloran @ 2020-04-02  3:50 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Mahesh Salgaonkar,
	Masahiro Yamada, Paul Mackerras, Mauro Carvalho Chehab, Ira Weiny,
	Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
	Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
	Alastair D'Silva, Andrew Donnellan, Arnd Bergmann, Greg Kurz,
	Nicholas Piggin, Cédric Le Goater, Dan Williams,
	Hari Bathini, Linux MM, Greg Kroah-Hartman,
	Linux Kernel Mailing List, Vishal Verma, Frederic Barrat,
	Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <87imiituxm.fsf@mpe.ellerman.id.au>

On Thu, Apr 2, 2020 at 2:42 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> "Alastair D'Silva" <alastair@d-silva.org> writes:
> >> -----Original Message-----
> >> From: Dan Williams <dan.j.williams@intel.com>
> >>
> >> On Sun, Mar 29, 2020 at 10:23 PM Alastair D'Silva <alastair@d-silva.org>
> >> wrote:
> >> >
> >> > *snip*
> >> Are OPAL calls similar to ACPI DSMs? I.e. methods for the OS to invoke
> >> platform firmware services? What's Skiboot?
> >>
> >
> > Yes, OPAL is the interface to firmware for POWER. Skiboot is the open-source (and only) implementation of OPAL.
>
>   https://github.com/open-power/skiboot
>
> In particular the tokens for calls are defined here:
>
>   https://github.com/open-power/skiboot/blob/master/include/opal-api.h#L220
>
> And you can grep for the token to find the implementation:
>
>   https://github.com/open-power/skiboot/blob/master/hw/npu2-opencapi.c#L2328

I'm not sure I'd encourage anyone to read npu2-opencapi.c. I find it
hard enough to follow even with access to the workbooks.

There's an OPAL call API reference here:
http://open-power.github.io/skiboot/doc/opal-api/index.html

Oliver

^ permalink raw reply

* RE: [PATCH v4 00/25] Add support for OpenCAPI Persistent Memory devices
From: Michael Ellerman @ 2020-04-02  3:42 UTC (permalink / raw)
  To: Alastair D'Silva, 'Dan Williams'
  Cc: 'Madhavan Srinivasan', 'Alexey Kardashevskiy',
	'Mahesh Salgaonkar', 'Masahiro Yamada',
	'Oliver O'Halloran', 'Mauro Carvalho Chehab',
	'Ira Weiny', 'Rob Herring', 'Dave Jiang',
	'linux-nvdimm', 'Aneesh Kumar K . V',
	'Krzysztof Kozlowski', 'Anju T Sudhakar',
	'Andrew Donnellan', 'Arnd Bergmann',
	'Greg Kurz', 'Nicholas Piggin',
	'Cédric Le Goater', 'Thomas Gleixner',
	'Hari Bathini', 'Linux MM',
	'Greg Kroah-Hartman', 'Linux Kernel Mailing List',
	'Vishal Verma', 'Frederic Barrat',
	'Paul Mackerras', 'Andrew Morton',
	'linuxppc-dev', 'David S. Miller'
In-Reply-To: <2d6901d60877$16aa7a90$43ff6fb0$@d-silva.org>

"Alastair D'Silva" <alastair@d-silva.org> writes:
>> -----Original Message-----
>> From: Dan Williams <dan.j.williams@intel.com>
>> 
>> On Sun, Mar 29, 2020 at 10:23 PM Alastair D'Silva <alastair@d-silva.org>
>> wrote:
>> >
>> > This series adds support for OpenCAPI Persistent Memory devices on
>> > bare metal (arch/powernv), exposing them as nvdimms so that we can
>> > make use of the existing infrastructure. There already exists a driver
>> > for the same devices abstracted through PowerVM (arch/pseries):
>> > arch/powerpc/platforms/pseries/papr_scm.c
>> >
>> > These devices are connected via OpenCAPI, and present as LPC (lowest
>> coherence point) memory to the system, practically, that means that
>> memory on these cards could be treated as conventional, cache-coherent
>> memory.
>> >
>> > Since the devices are connected via OpenCAPI, they are not enumerated
>> via ACPI. Instead, OpenCAPI links present as pseudo-PCI bridges, with
>> devices below them.
>> >
>> > This series introduces a driver that exposes the memory on these cards as
>> nvdimms, with each card getting it's own bus. This is somewhat complicated
>> by the fact that the cards do not have out of band persistent storage for
>> metadata, so 1 SECTION_SIZE's (see SPARSEMEM) worth of storage is carved
>> out of the top of the card storage to implement the ndctl_config_* calls.
>> 
>> Is it really tied to section-size? Can't that change based on the configured
>> page-size? It's not clear to me why that would be the choice, but I'll dig into
>> the implementation.
>> 
>
> I had tried using PAGE_SIZE, but ran into problems carving off just 1 page and handing it to the kernel, while leaving the rest as pmem. That was a while ago though, so maybe I should retry it.
>
>> > The driver is not responsible for configuring the NPU (NVLink Processing
>> Unit) BARs to map the LPC memory from the card into the system's physical
>> address space, instead, it requests this to be done via OPAL calls (typically
>> implemented by Skiboot).
>> 
>> Are OPAL calls similar to ACPI DSMs? I.e. methods for the OS to invoke
>> platform firmware services? What's Skiboot?
>> 
>
> Yes, OPAL is the interface to firmware for POWER. Skiboot is the open-source (and only) implementation of OPAL.

  https://github.com/open-power/skiboot

In particular the tokens for calls are defined here:

  https://github.com/open-power/skiboot/blob/master/include/opal-api.h#L220

And you can grep for the token to find the implementation:

  https://github.com/open-power/skiboot/blob/master/hw/npu2-opencapi.c#L2328


cheers

^ permalink raw reply

* Re: [PATCH v5 1/4] powerpc/papr_scm: Fetch nvdimm health information from PHYP
From: Dan Williams @ 2020-04-02  3:08 UTC (permalink / raw)
  To: Vaibhav Jain
  Cc: Alastair D'Silva, linux-nvdimm, Aneesh Kumar K . V,
	Jeff Moyer, Oliver O'Halloran, Vishal Verma, Michael Ellerman,
	linuxppc-dev
In-Reply-To: <20200331143229.306718-2-vaibhav@linux.ibm.com>

On Tue, Mar 31, 2020 at 7:33 AM Vaibhav Jain <vaibhav@linux.ibm.com> wrote:
>
> Implement support for fetching nvdimm health information via
> H_SCM_HEALTH hcall as documented in Ref[1]. The hcall returns a pair
> of 64-bit big-endian integers which are then stored in 'struct
> papr_scm_priv' and subsequently partially exposed to user-space via
> newly introduced dimm specific attribute 'papr_flags'. Also a new asm
> header named 'papr-scm.h' is added that describes the interface
> between PHYP and guest kernel.
>
> Following flags are reported via 'papr_flags' sysfs attribute contents
> of which are space separated string flags indicating various nvdimm
> states:
>
>  * "not_armed"  : Indicating that nvdimm contents wont survive a power
>                    cycle.

s/wont/will not/

>  * "save_fail"  : Indicating that nvdimm contents couldn't be flushed
>                    during last shutdown event.

In the nfit definition this description is "flush_fail". The
"save_fail" flag was specific to hybrid devices that don't have
persistent media and instead scuttle away data from DRAM to flash on
power-failure.

>  * "restore_fail": Indicating that nvdimm contents couldn't be restored
>                    during dimm initialization.
>  * "encrypted"  : Dimm contents are encrypted.

This does not seem like a health flag to me, have you considered the
libnvdimm security interface for this indicator?

>  * "smart_notify": There is health event for the nvdimm.

Are you also going to signal the sysfs attribute when this event happens?

>  * "scrubbed"   : Indicating that contents of the nvdimm have been
>                    scrubbed.

This one seems odd to me what does it mean if it is not set? What does
it mean if a new scrub has been launched. Basically, is there value in
exposing this state?

>  * "locked"     : Indicating that nvdimm contents cant be modified
>                    until next power cycle.

There is the generic NDD_LOCKED flag, can you use that? ...and in
general I wonder if we should try to unify all the common papr_scm and
nfit health flags in a generic location. It will already be the case
the ndctl needs to look somewhere papr specific for this data maybe it
all should have been generic from the beginning.


In any event, can you also add this content to a new
Documentation/ABI/testing/sysfs-bus-papr? See sysfs-bus-nfit for
comparison.

>
> [1]: commit 58b278f568f0 ("powerpc: Provide initial documentation for
> PAPR hcalls")
>
> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> ---
> Changelog:
>
> v4..v5 : None
>
> v3..v4 : None
>
> v2..v3 : Removed PAPR_SCM_DIMM_HEALTH_NON_CRITICAL as a condition for
>          NVDIMM unarmed [Aneesh]
>
> v1..v2 : New patch in the series.
> ---
>  arch/powerpc/include/asm/papr_scm.h       |  48 ++++++++++
>  arch/powerpc/platforms/pseries/papr_scm.c | 105 +++++++++++++++++++++-
>  2 files changed, 151 insertions(+), 2 deletions(-)
>  create mode 100644 arch/powerpc/include/asm/papr_scm.h
>
> diff --git a/arch/powerpc/include/asm/papr_scm.h b/arch/powerpc/include/asm/papr_scm.h
> new file mode 100644
> index 000000000000..868d3360f56a
> --- /dev/null
> +++ b/arch/powerpc/include/asm/papr_scm.h
> @@ -0,0 +1,48 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Structures and defines needed to manage nvdimms for spapr guests.
> + */
> +#ifndef _ASM_POWERPC_PAPR_SCM_H_
> +#define _ASM_POWERPC_PAPR_SCM_H_
> +
> +#include <linux/types.h>
> +#include <asm/bitsperlong.h>
> +
> +/* DIMM health bitmap bitmap indicators */
> +/* SCM device is unable to persist memory contents */
> +#define PAPR_SCM_DIMM_UNARMED                  PPC_BIT(0)
> +/* SCM device failed to persist memory contents */
> +#define PAPR_SCM_DIMM_SHUTDOWN_DIRTY           PPC_BIT(1)
> +/* SCM device contents are persisted from previous IPL */
> +#define PAPR_SCM_DIMM_SHUTDOWN_CLEAN           PPC_BIT(2)
> +/* SCM device contents are not persisted from previous IPL */
> +#define PAPR_SCM_DIMM_EMPTY                    PPC_BIT(3)
> +/* SCM device memory life remaining is critically low */
> +#define PAPR_SCM_DIMM_HEALTH_CRITICAL          PPC_BIT(4)
> +/* SCM device will be garded off next IPL due to failure */
> +#define PAPR_SCM_DIMM_HEALTH_FATAL             PPC_BIT(5)
> +/* SCM contents cannot persist due to current platform health status */
> +#define PAPR_SCM_DIMM_HEALTH_UNHEALTHY         PPC_BIT(6)
> +/* SCM device is unable to persist memory contents in certain conditions */
> +#define PAPR_SCM_DIMM_HEALTH_NON_CRITICAL      PPC_BIT(7)
> +/* SCM device is encrypted */
> +#define PAPR_SCM_DIMM_ENCRYPTED                        PPC_BIT(8)
> +/* SCM device has been scrubbed and locked */
> +#define PAPR_SCM_DIMM_SCRUBBED_AND_LOCKED      PPC_BIT(9)
> +
> +/* Bits status indicators for health bitmap indicating unarmed dimm */
> +#define PAPR_SCM_DIMM_UNARMED_MASK (PAPR_SCM_DIMM_UNARMED |    \
> +                                       PAPR_SCM_DIMM_HEALTH_UNHEALTHY)
> +
> +/* Bits status indicators for health bitmap indicating unflushed dimm */
> +#define PAPR_SCM_DIMM_BAD_SHUTDOWN_MASK (PAPR_SCM_DIMM_SHUTDOWN_DIRTY)
> +
> +/* Bits status indicators for health bitmap indicating unrestored dimm */
> +#define PAPR_SCM_DIMM_BAD_RESTORE_MASK  (PAPR_SCM_DIMM_EMPTY)
> +
> +/* Bit status indicators for smart event notification */
> +#define PAPR_SCM_DIMM_SMART_EVENT_MASK (PAPR_SCM_DIMM_HEALTH_CRITICAL | \
> +                                          PAPR_SCM_DIMM_HEALTH_FATAL | \
> +                                          PAPR_SCM_DIMM_HEALTH_UNHEALTHY)
> +
> +#endif
> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
> index 0b4467e378e5..aaf2e4ab1f75 100644
> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> @@ -14,6 +14,7 @@
>  #include <linux/delay.h>
>
>  #include <asm/plpar_wrappers.h>
> +#include <asm/papr_scm.h>
>
>  #define BIND_ANY_ADDR (~0ul)
>
> @@ -39,6 +40,13 @@ struct papr_scm_priv {
>         struct resource res;
>         struct nd_region *region;
>         struct nd_interleave_set nd_set;
> +
> +       /* Protect dimm data from concurrent access */
> +       struct mutex dimm_mutex;
> +
> +       /* Health information for the dimm */
> +       __be64 health_bitmap;
> +       __be64 health_bitmap_valid;
>  };
>
>  static int drc_pmem_bind(struct papr_scm_priv *p)
> @@ -144,6 +152,35 @@ static int drc_pmem_query_n_bind(struct papr_scm_priv *p)
>         return drc_pmem_bind(p);
>  }
>
> +static int drc_pmem_query_health(struct papr_scm_priv *p)
> +{
> +       unsigned long ret[PLPAR_HCALL_BUFSIZE];
> +       int64_t rc;
> +
> +       rc = plpar_hcall(H_SCM_HEALTH, ret, p->drc_index);
> +       if (rc != H_SUCCESS) {
> +               dev_err(&p->pdev->dev,
> +                        "Failed to query health information, Err:%lld\n", rc);
> +               return -ENXIO;
> +       }
> +
> +       /* Protect modifications to papr_scm_priv with the mutex */
> +       rc = mutex_lock_interruptible(&p->dimm_mutex);
> +       if (rc)
> +               return rc;
> +
> +       /* Store the retrieved health information in dimm platform data */
> +       p->health_bitmap = ret[0];
> +       p->health_bitmap_valid = ret[1];
> +
> +       dev_dbg(&p->pdev->dev,
> +               "Queried dimm health info. Bitmap:0x%016llx Mask:0x%016llx\n",
> +               be64_to_cpu(p->health_bitmap),
> +               be64_to_cpu(p->health_bitmap_valid));
> +
> +       mutex_unlock(&p->dimm_mutex);
> +       return 0;
> +}
>
>  static int papr_scm_meta_get(struct papr_scm_priv *p,
>                              struct nd_cmd_get_config_data_hdr *hdr)
> @@ -304,6 +341,67 @@ static inline int papr_scm_node(int node)
>         return min_node;
>  }
>
> +static ssize_t papr_flags_show(struct device *dev,
> +                               struct device_attribute *attr, char *buf)
> +{
> +       struct nvdimm *dimm = to_nvdimm(dev);
> +       struct papr_scm_priv *p = nvdimm_provider_data(dimm);
> +       __be64 health;
> +       int rc;
> +
> +       rc = drc_pmem_query_health(p);
> +       if (rc)
> +               return rc;

This attribute is user readable which means userspace can trigger and
ongoing stream of device requests. Is that desired? The nfit driver
caches this flag data, and limits attributes with device side-effects
to root-only. The expectation is that ndctl submits commands to
retrieve the up to date state especially because that payload has
other interesting data like temperature that can't be cached.

> +
> +       /* Protect against modifications to papr_scm_priv with the mutex */

What papr_scm_priv modifications are you worried about because none of
the below looks like it needs to be locked, and papr_scm_priv had
better be stable otherwise the above usages would have crashed.

> +       rc = mutex_lock_interruptible(&p->dimm_mutex);
> +       if (rc)
> +               return rc;
> +
> +       health = p->health_bitmap & p->health_bitmap_valid;
> +
> +       /* Check for various masks in bitmap and set the buffer */
> +       if (health & PAPR_SCM_DIMM_UNARMED_MASK)
> +               rc += sprintf(buf, "not_armed ");
> +
> +       if (health & PAPR_SCM_DIMM_BAD_SHUTDOWN_MASK)
> +               rc += sprintf(buf + rc, "save_fail ");
> +
> +       if (health & PAPR_SCM_DIMM_BAD_RESTORE_MASK)
> +               rc += sprintf(buf + rc, "restore_fail ");
> +
> +       if (health & PAPR_SCM_DIMM_ENCRYPTED)
> +               rc += sprintf(buf + rc, "encrypted ");
> +
> +       if (health & PAPR_SCM_DIMM_SMART_EVENT_MASK)
> +               rc += sprintf(buf + rc, "smart_notify ");
> +
> +       if (health & PAPR_SCM_DIMM_SCRUBBED_AND_LOCKED)
> +               rc += sprintf(buf + rc, "scrubbed locked ");

See above about whether we event need this here...

> +
> +       if (rc > 0)
> +               rc += sprintf(buf + rc, "\n");
> +
> +       mutex_unlock(&p->dimm_mutex);
> +       return rc;
> +}
> +DEVICE_ATTR_RO(papr_flags);

Rather than name this attribute "papr_flags", I'd prefer "papr/flags".
I.e. create a "papr" sub-directory...

> +
> +/* papr_scm specific dimm attributes */
> +static struct attribute *papr_scm_nd_attributes[] = {
> +       &dev_attr_papr_flags.attr,
> +       NULL,
> +};
> +
> +static struct attribute_group papr_scm_nd_attribute_group = {

...here:

       .name = "papr",

> +       .attrs = papr_scm_nd_attributes,
> +};
> +

With the continued discussions this is going to need I hope you
understand that I consider this v5.8 material.

> +static const struct attribute_group *papr_scm_dimm_attr_groups[] = {
> +       &papr_scm_nd_attribute_group,
> +       NULL,
> +};
> +
>  static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
>  {
>         struct device *dev = &p->pdev->dev;
> @@ -330,8 +428,8 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
>         dimm_flags = 0;
>         set_bit(NDD_ALIASING, &dimm_flags);
>
> -       p->nvdimm = nvdimm_create(p->bus, p, NULL, dimm_flags,
> -                                 PAPR_SCM_DIMM_CMD_MASK, 0, NULL);
> +       p->nvdimm = nvdimm_create(p->bus, p, papr_scm_dimm_attr_groups,
> +                                 dimm_flags, PAPR_SCM_DIMM_CMD_MASK, 0, NULL);
>         if (!p->nvdimm) {
>                 dev_err(dev, "Error creating DIMM object for %pOF\n", p->dn);
>                 goto err;
> @@ -415,6 +513,9 @@ static int papr_scm_probe(struct platform_device *pdev)
>         if (!p)
>                 return -ENOMEM;
>
> +       /* Initialize the dimm mutex */
> +       mutex_init(&p->dimm_mutex);
> +
>         /* optional DT properties */
>         of_property_read_u32(dn, "ibm,metadata-size", &metadata_size);
>
> --
> 2.25.1
>

^ permalink raw reply

* Re: [PATCH v4 19/25] nvdimm/ocxl: Forward events to userspace
From: Dan Williams @ 2020-04-02  2:08 UTC (permalink / raw)
  To: Alastair D'Silva
  Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
	Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
	Rob Herring, Dave Jiang, linux-nvdimm, Aneesh Kumar K . V,
	Krzysztof Kozlowski, Anju T Sudhakar, Mahesh Salgaonkar,
	Andrew Donnellan, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
	Cédric Le Goater, Thomas Gleixner, Hari Bathini, Linux MM,
	Greg Kroah-Hartman, Linux Kernel Mailing List, Vishal Verma,
	Frederic Barrat, Paul Mackerras, Andrew Morton, linuxppc-dev,
	David S. Miller
In-Reply-To: <20200327071202.2159885-20-alastair@d-silva.org>

On Tue, Mar 31, 2020 at 1:59 AM Alastair D'Silva <alastair@d-silva.org> wrote:
>
> Some of the interrupts that the card generates are better handled
> by the userspace daemon, in particular:
> Controller Hardware/Firmware Fatal
> Controller Dump Available
> Error Log available
>
> This patch allows a userspace application to register an eventfd with
> the driver via SCM_IOCTL_EVENTFD to receive notifications of these
> interrupts.
>
> Userspace can then identify what events have occurred by calling
> SCM_IOCTL_EVENT_CHECK and checking against the SCM_IOCTL_EVENT_FOO
> masks.

The amount new ioctl's in this driver is too high, it seems much of
this data can be exported via sysfs attributes which are more
maintainable that ioctls. Then sysfs also has the ability to signal
events on sysfs attributes, see sys_notify_dirent.

Can you step back and review the ABI exposure of the driver and what
can be moved to sysfs? If you need to have bus specific attributes
ordered underneath the libnvdimm generic attributes you can create a
sysfs attribute subdirectory.

In general a roadmap document of all the proposed ABI is needed to
make sure it is both sufficient and necessary. See the libnvdimm
document that introduced the initial libnvdimm ABI:

https://www.kernel.org/doc/Documentation/nvdimm/nvdimm.txt

>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
>  drivers/nvdimm/ocxl/main.c     | 220 +++++++++++++++++++++++++++++++++
>  drivers/nvdimm/ocxl/ocxlpmem.h |   4 +
>  include/uapi/nvdimm/ocxlpmem.h |  12 ++
>  3 files changed, 236 insertions(+)
>
> diff --git a/drivers/nvdimm/ocxl/main.c b/drivers/nvdimm/ocxl/main.c
> index 0040fc09cceb..cb6cdc9eb899 100644
> --- a/drivers/nvdimm/ocxl/main.c
> +++ b/drivers/nvdimm/ocxl/main.c
> @@ -10,6 +10,7 @@
>  #include <misc/ocxl.h>
>  #include <linux/delay.h>
>  #include <linux/ndctl.h>
> +#include <linux/eventfd.h>
>  #include <linux/fs.h>
>  #include <linux/mm_types.h>
>  #include <linux/memory_hotplug.h>
> @@ -301,8 +302,19 @@ static void free_ocxlpmem(struct ocxlpmem *ocxlpmem)
>  {
>         int rc;
>
> +       // Disable doorbells
> +       (void)ocxl_global_mmio_set64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_CHIEC,
> +                                    OCXL_LITTLE_ENDIAN,
> +                                    GLOBAL_MMIO_CHI_ALL);
> +
>         free_minor(ocxlpmem);
>
> +       if (ocxlpmem->irq_addr[1])
> +               iounmap(ocxlpmem->irq_addr[1]);
> +
> +       if (ocxlpmem->irq_addr[0])
> +               iounmap(ocxlpmem->irq_addr[0]);
> +
>         if (ocxlpmem->ocxl_context) {
>                 rc = ocxl_context_detach(ocxlpmem->ocxl_context);
>                 if (rc == -EBUSY)
> @@ -398,6 +410,11 @@ static int file_release(struct inode *inode, struct file *file)
>  {
>         struct ocxlpmem *ocxlpmem = file->private_data;
>
> +       if (ocxlpmem->ev_ctx) {
> +               eventfd_ctx_put(ocxlpmem->ev_ctx);
> +               ocxlpmem->ev_ctx = NULL;
> +       }
> +
>         ocxlpmem_put(ocxlpmem);
>         return 0;
>  }
> @@ -928,6 +945,52 @@ static int ioctl_controller_stats(struct ocxlpmem *ocxlpmem,
>         return rc;
>  }
>
> +static int ioctl_eventfd(struct ocxlpmem *ocxlpmem,
> +                        struct ioctl_ocxlpmem_eventfd __user *uarg)
> +{
> +       struct ioctl_ocxlpmem_eventfd args;
> +
> +       if (copy_from_user(&args, uarg, sizeof(args)))
> +               return -EFAULT;
> +
> +       if (ocxlpmem->ev_ctx)
> +               return -EBUSY;
> +
> +       ocxlpmem->ev_ctx = eventfd_ctx_fdget(args.eventfd);
> +       if (IS_ERR(ocxlpmem->ev_ctx))
> +               return PTR_ERR(ocxlpmem->ev_ctx);
> +
> +       return 0;
> +}
> +
> +static int ioctl_event_check(struct ocxlpmem *ocxlpmem, u64 __user *uarg)
> +{
> +       u64 val = 0;
> +       int rc;
> +       u64 chi = 0;
> +
> +       rc = ocxlpmem_chi(ocxlpmem, &chi);
> +       if (rc < 0)
> +               return rc;
> +
> +       if (chi & GLOBAL_MMIO_CHI_ELA)
> +               val |= IOCTL_OCXLPMEM_EVENT_ERROR_LOG_AVAILABLE;
> +
> +       if (chi & GLOBAL_MMIO_CHI_CDA)
> +               val |= IOCTL_OCXLPMEM_EVENT_CONTROLLER_DUMP_AVAILABLE;
> +
> +       if (chi & GLOBAL_MMIO_CHI_CFFS)
> +               val |= IOCTL_OCXLPMEM_EVENT_FIRMWARE_FATAL;
> +
> +       if (chi & GLOBAL_MMIO_CHI_CHFS)
> +               val |= IOCTL_OCXLPMEM_EVENT_HARDWARE_FATAL;
> +
> +       if (copy_to_user((u64 __user *)uarg, &val, sizeof(val)))
> +               return -EFAULT;
> +
> +       return rc;
> +}
> +
>  static long file_ioctl(struct file *file, unsigned int cmd, unsigned long args)
>  {
>         struct ocxlpmem *ocxlpmem = file->private_data;
> @@ -956,6 +1019,15 @@ static long file_ioctl(struct file *file, unsigned int cmd, unsigned long args)
>                 rc = ioctl_controller_stats(ocxlpmem,
>                                             (struct ioctl_ocxlpmem_controller_stats __user *)args);
>                 break;
> +
> +       case IOCTL_OCXLPMEM_EVENTFD:
> +               rc = ioctl_eventfd(ocxlpmem,
> +                                  (struct ioctl_ocxlpmem_eventfd __user *)args);
> +               break;
> +
> +       case IOCTL_OCXLPMEM_EVENT_CHECK:
> +               rc = ioctl_event_check(ocxlpmem, (u64 __user *)args);
> +               break;
>         }
>
>         return rc;
> @@ -1109,6 +1181,148 @@ static void dump_error_log(struct ocxlpmem *ocxlpmem)
>         kfree(buf);
>  }
>
> +static irqreturn_t imn0_handler(void *private)
> +{
> +       struct ocxlpmem *ocxlpmem = private;
> +       u64 chi = 0;
> +
> +       (void)ocxlpmem_chi(ocxlpmem, &chi);
> +
> +       if (chi & GLOBAL_MMIO_CHI_ELA) {
> +               dev_warn(&ocxlpmem->dev, "Error log is available\n");
> +
> +               if (ocxlpmem->ev_ctx)
> +                       eventfd_signal(ocxlpmem->ev_ctx, 1);
> +       }
> +
> +       if (chi & GLOBAL_MMIO_CHI_CDA) {
> +               dev_warn(&ocxlpmem->dev, "Controller dump is available\n");
> +
> +               if (ocxlpmem->ev_ctx)
> +                       eventfd_signal(ocxlpmem->ev_ctx, 1);
> +       }
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t imn1_handler(void *private)
> +{
> +       struct ocxlpmem *ocxlpmem = private;
> +       u64 chi = 0;
> +
> +       (void)ocxlpmem_chi(ocxlpmem, &chi);
> +
> +       if (chi & (GLOBAL_MMIO_CHI_CFFS | GLOBAL_MMIO_CHI_CHFS)) {
> +               dev_err(&ocxlpmem->dev,
> +                       "Controller status is fatal, chi=0x%llx, going offline\n",
> +                       chi);
> +
> +               if (ocxlpmem->nvdimm_bus) {
> +                       nvdimm_bus_unregister(ocxlpmem->nvdimm_bus);
> +                       ocxlpmem->nvdimm_bus = NULL;
> +               }
> +
> +               if (ocxlpmem->ev_ctx)
> +                       eventfd_signal(ocxlpmem->ev_ctx, 1);
> +       }
> +
> +       return IRQ_HANDLED;
> +}
> +
> +/**
> + * ocxlpmem_setup_irq() - Set up the IRQs for the OpenCAPI Persistent Memory device
> + * @ocxlpmem: the device metadata
> + * Return: 0 on success, negative on failure
> + */
> +static int setup_irqs(struct ocxlpmem *ocxlpmem)
> +{
> +       int rc;
> +       u64 irq_addr;
> +
> +       rc = ocxl_afu_irq_alloc(ocxlpmem->ocxl_context,
> +                               &ocxlpmem->irq_id[0]);
> +       if (rc)
> +               return rc;
> +
> +       rc = ocxl_irq_set_handler(ocxlpmem->ocxl_context, ocxlpmem->irq_id[0],
> +                                 imn0_handler, NULL, ocxlpmem);
> +
> +       irq_addr = ocxl_afu_irq_get_addr(ocxlpmem->ocxl_context,
> +                                        ocxlpmem->irq_id[0]);
> +       if (!irq_addr)
> +               return -EFAULT;
> +
> +       ocxlpmem->irq_addr[0] = ioremap(irq_addr, PAGE_SIZE);
> +       if (!ocxlpmem->irq_addr[0])
> +               return -ENODEV;
> +
> +       rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_IMA0_OHP,
> +                                     OCXL_LITTLE_ENDIAN,
> +                                     (u64)ocxlpmem->irq_addr[0]);
> +       if (rc)
> +               goto out_irq0;
> +
> +       rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_IMA0_CFP,
> +                                     OCXL_LITTLE_ENDIAN, 0);
> +       if (rc)
> +               goto out_irq0;
> +
> +       rc = ocxl_afu_irq_alloc(ocxlpmem->ocxl_context, &ocxlpmem->irq_id[1]);
> +       if (rc)
> +               goto out_irq0;
> +
> +       rc = ocxl_irq_set_handler(ocxlpmem->ocxl_context, ocxlpmem->irq_id[1],
> +                                 imn1_handler, NULL, ocxlpmem);
> +       if (rc)
> +               goto out_irq0;
> +
> +       irq_addr = ocxl_afu_irq_get_addr(ocxlpmem->ocxl_context,
> +                                        ocxlpmem->irq_id[1]);
> +       if (!irq_addr) {
> +               rc = -EFAULT;
> +               goto out_irq0;
> +       }
> +
> +       ocxlpmem->irq_addr[1] = ioremap(irq_addr, PAGE_SIZE);
> +       if (!ocxlpmem->irq_addr[1]) {
> +               rc = -ENODEV;
> +               goto out_irq0;
> +       }
> +
> +       rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_IMA1_OHP,
> +                                     OCXL_LITTLE_ENDIAN,
> +                                     (u64)ocxlpmem->irq_addr[1]);
> +       if (rc)
> +               goto out_irq1;
> +
> +       rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_IMA1_CFP,
> +                                     OCXL_LITTLE_ENDIAN, 0);
> +       if (rc)
> +               goto out_irq1;
> +
> +       // Enable doorbells
> +       rc = ocxl_global_mmio_set64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_CHIE,
> +                                   OCXL_LITTLE_ENDIAN,
> +                                   GLOBAL_MMIO_CHI_ELA |
> +                                   GLOBAL_MMIO_CHI_CDA |
> +                                   GLOBAL_MMIO_CHI_CFFS |
> +                                   GLOBAL_MMIO_CHI_CHFS);
> +       if (rc)
> +               goto out_irq1;
> +
> +       return 0;
> +
> +out_irq1:
> +       iounmap(ocxlpmem->irq_addr[1]);
> +       ocxlpmem->irq_addr[1] = NULL;
> +
> +out_irq0:
> +       iounmap(ocxlpmem->irq_addr[0]);
> +       ocxlpmem->irq_addr[0] = NULL;
> +
> +       return rc;
> +}
> +
>  /**
>   * probe_function0() - Set up function 0 for an OpenCAPI persistent memory device
>   * This is important as it enables templates higher than 0 across all other
> @@ -1212,6 +1426,12 @@ static int probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>                 goto err;
>         }
>
> +       rc = setup_irqs(ocxlpmem);
> +       if (rc) {
> +               dev_err(&pdev->dev, "Could not set up OCXL IRQs\n");
> +               goto err;
> +       }
> +
>         rc = setup_command_metadata(ocxlpmem);
>         if (rc) {
>                 dev_err(&pdev->dev, "Could not read command metadata\n");
> diff --git a/drivers/nvdimm/ocxl/ocxlpmem.h b/drivers/nvdimm/ocxl/ocxlpmem.h
> index ee3bd651f254..01721596f982 100644
> --- a/drivers/nvdimm/ocxl/ocxlpmem.h
> +++ b/drivers/nvdimm/ocxl/ocxlpmem.h
> @@ -106,6 +106,9 @@ struct ocxlpmem {
>         struct pci_dev *pdev;
>         struct cdev cdev;
>         struct ocxl_fn *ocxl_fn;
> +#define SCM_IRQ_COUNT 2
> +       int irq_id[SCM_IRQ_COUNT];
> +       void __iomem *irq_addr[SCM_IRQ_COUNT];
>         struct nd_interleave_set nd_set;
>         struct nvdimm_bus_descriptor bus_desc;
>         struct nvdimm_bus *nvdimm_bus;
> @@ -117,6 +120,7 @@ struct ocxlpmem {
>         struct nd_region *nd_region;
>         char fw_version[8 + 1];
>         u32 timeouts[ADMIN_COMMAND_MAX + 1];
> +       struct eventfd_ctx *ev_ctx;
>         u32 max_controller_dump_size;
>         u16 scm_revision; // major/minor
>         u8 readiness_timeout;  /* The worst case time (in seconds) that the host
> diff --git a/include/uapi/nvdimm/ocxlpmem.h b/include/uapi/nvdimm/ocxlpmem.h
> index ca3a7098fa9d..d573bd307e35 100644
> --- a/include/uapi/nvdimm/ocxlpmem.h
> +++ b/include/uapi/nvdimm/ocxlpmem.h
> @@ -71,6 +71,16 @@ struct ioctl_ocxlpmem_controller_stats {
>         __u64 fast_write_count;
>  };
>
> +struct ioctl_ocxlpmem_eventfd {
> +       __s32 eventfd;
> +       __u32 reserved;
> +};
> +
> +#define IOCTL_OCXLPMEM_EVENT_CONTROLLER_DUMP_AVAILABLE (1ULL << (0))
> +#define IOCTL_OCXLPMEM_EVENT_ERROR_LOG_AVAILABLE       (1ULL << (1))
> +#define IOCTL_OCXLPMEM_EVENT_HARDWARE_FATAL            (1ULL << (2))
> +#define IOCTL_OCXLPMEM_EVENT_FIRMWARE_FATAL            (1ULL << (3))
> +
>  /* ioctl numbers */
>  #define OCXLPMEM_MAGIC 0xCA
>  /* OpenCAPI Persistent memory devices */
> @@ -79,5 +89,7 @@ struct ioctl_ocxlpmem_controller_stats {
>  #define IOCTL_OCXLPMEM_CONTROLLER_DUMP_DATA            _IOWR(OCXLPMEM_MAGIC, 0x32, struct ioctl_ocxlpmem_controller_dump_data)
>  #define IOCTL_OCXLPMEM_CONTROLLER_DUMP_COMPLETE                _IO(OCXLPMEM_MAGIC, 0x33)
>  #define IOCTL_OCXLPMEM_CONTROLLER_STATS                        _IO(OCXLPMEM_MAGIC, 0x34)
> +#define IOCTL_OCXLPMEM_EVENTFD                         _IOW(OCXLPMEM_MAGIC, 0x35, struct ioctl_ocxlpmem_eventfd)
> +#define IOCTL_OCXLPMEM_EVENT_CHECK                     _IOR(OCXLPMEM_MAGIC, 0x36, __u64)
>
>  #endif /* _UAPI_OCXL_SCM_H */
> --
> 2.24.1
>

^ permalink raw reply

* Re: [PATCH v4 08/25] ocxl: Emit a log message showing how much LPC memory was detected
From: Joe Perches @ 2020-04-02  1:29 UTC (permalink / raw)
  To: Dan Williams, Alastair D'Silva
  Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
	Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
	Rob Herring, Dave Jiang, linux-nvdimm, Aneesh Kumar K . V,
	Krzysztof Kozlowski, Anju T Sudhakar, Mahesh Salgaonkar,
	Andrew Donnellan, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
	Cédric Le Goater, Thomas Gleixner, Hari Bathini, Linux MM,
	Greg Kroah-Hartman, Linux Kernel Mailing List, Vishal Verma,
	Frederic Barrat, Paul Mackerras, Andrew Morton, linuxppc-dev,
	David S. Miller
In-Reply-To: <CAPcyv4j4_owxEVjanwH5TiuMMJB3CaMannDzpXnaHedX7LuarQ@mail.gmail.com>

On Wed, 2020-04-01 at 01:49 -0700, Dan Williams wrote:
> On Sun, Mar 29, 2020 at 10:23 PM Alastair D'Silva <alastair@d-silva.org> wrote:
> > This patch emits a message showing how much LPC memory & special purpose
> > memory was detected on an OCXL device.
[]
> > diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
[]
> > @@ -568,6 +568,10 @@ static int read_afu_lpc_memory_info(struct pci_dev *dev,
> >                 afu->special_purpose_mem_size =
> >                         total_mem_size - lpc_mem_size;
> >         }
> > +
> > +       dev_info(&dev->dev, "Probed LPC memory of %#llx bytes and special purpose memory of %#llx bytes\n",
> > +                afu->lpc_mem_size, afu->special_purpose_mem_size);
> 
> A patch for a single log message is too fine grained for my taste,
> let's squash this into another patch in the series.

Is the granularity of lpc_mem_size actually bytes?
Might this be better as KiB or something using functions

Maybe something like:

unsigned long si_val(unsigned long val)
{
	static const char units[] = "BKMGTPE";
	const char *unit = units;

	while (!(val & 1023) && unit[1]) {
		val >>= 10;
		unit++;
	}

	return val;
}

char si_type(unsigned long val)
{
	static const char units[] = "BKMGTPE";
	const char *unit = units;

	while (!(val & 1023) && unit[1]) {
		val >>= 10;
		unit++;
	}

	return *unit;
}

so this could be something like:

       dev_info(&dev->dev, "Probed LPC memory of %#llu%c and special purpose memory of %#llu%c\n",
                si_val(afu->lpc_mem_size), si_type(afu->lpc_mem_size),
		si_val(afu->special_purpose_mem_size), si_type(afu->special_purpose_mem_size));




^ permalink raw reply

* Re: [PATCH v4 15/25] nvdimm/ocxl: Register a character device for userspace to interact with
From: Dan Williams @ 2020-04-02  0:27 UTC (permalink / raw)
  To: Alastair D'Silva
  Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
	Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
	Rob Herring, Dave Jiang, linux-nvdimm, Aneesh Kumar K . V,
	Krzysztof Kozlowski, Anju T Sudhakar, Mahesh Salgaonkar,
	Andrew Donnellan, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
	Cédric Le Goater, Thomas Gleixner, Hari Bathini, Linux MM,
	Greg Kroah-Hartman, Linux Kernel Mailing List, Vishal Verma,
	Frederic Barrat, Paul Mackerras, Andrew Morton, linuxppc-dev,
	David S. Miller
In-Reply-To: <20200327071202.2159885-16-alastair@d-silva.org>

On Sun, Mar 29, 2020 at 10:53 PM Alastair D'Silva <alastair@d-silva.org> wrote:
>
> This patch introduces a character device (/dev/ocxlpmemX) which further
> patches will use to interact with userspace, such as error logs,
> controller stats and card debug functionality.

This was asked earlier, but I'll reiterate, I do not see what
justifies an ocxlpmemX private device ABI vs routing through the
existing generic character ndbusX and nmemX character devices.

>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
>  drivers/nvdimm/ocxl/main.c     | 117 ++++++++++++++++++++++++++++++++-
>  drivers/nvdimm/ocxl/ocxlpmem.h |   2 +
>  2 files changed, 117 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvdimm/ocxl/main.c b/drivers/nvdimm/ocxl/main.c
> index 8db573036423..9b85fcd3f1c9 100644
> --- a/drivers/nvdimm/ocxl/main.c
> +++ b/drivers/nvdimm/ocxl/main.c
> @@ -10,6 +10,7 @@
>  #include <misc/ocxl.h>
>  #include <linux/delay.h>
>  #include <linux/ndctl.h>
> +#include <linux/fs.h>
>  #include <linux/mm_types.h>
>  #include <linux/memory_hotplug.h>
>  #include "ocxlpmem.h"
> @@ -356,6 +357,67 @@ static int ocxlpmem_register(struct ocxlpmem *ocxlpmem)
>         return device_register(&ocxlpmem->dev);
>  }
>
> +static void ocxlpmem_put(struct ocxlpmem *ocxlpmem)
> +{
> +       put_device(&ocxlpmem->dev);
> +}
> +
> +static struct ocxlpmem *ocxlpmem_get(struct ocxlpmem *ocxlpmem)
> +{
> +       return (!get_device(&ocxlpmem->dev)) ? NULL : ocxlpmem;
> +}
> +
> +static struct ocxlpmem *find_and_get_ocxlpmem(dev_t devno)
> +{
> +       struct ocxlpmem *ocxlpmem;
> +       int minor = MINOR(devno);
> +
> +       mutex_lock(&minors_idr_lock);
> +       ocxlpmem = idr_find(&minors_idr, minor);
> +       if (ocxlpmem)
> +               ocxlpmem_get(ocxlpmem);
> +       mutex_unlock(&minors_idr_lock);
> +
> +       return ocxlpmem;
> +}
> +
> +static int file_open(struct inode *inode, struct file *file)
> +{
> +       struct ocxlpmem *ocxlpmem;
> +
> +       ocxlpmem = find_and_get_ocxlpmem(inode->i_rdev);
> +       if (!ocxlpmem)
> +               return -ENODEV;
> +
> +       file->private_data = ocxlpmem;
> +       return 0;
> +}
> +
> +static int file_release(struct inode *inode, struct file *file)
> +{
> +       struct ocxlpmem *ocxlpmem = file->private_data;
> +
> +       ocxlpmem_put(ocxlpmem);
> +       return 0;
> +}
> +
> +static const struct file_operations fops = {
> +       .owner          = THIS_MODULE,
> +       .open           = file_open,
> +       .release        = file_release,
> +};
> +
> +/**
> + * create_cdev() - Create the chardev in /dev for the device
> + * @ocxlpmem: the SCM metadata
> + * Return: 0 on success, negative on failure
> + */
> +static int create_cdev(struct ocxlpmem *ocxlpmem)
> +{
> +       cdev_init(&ocxlpmem->cdev, &fops);
> +       return cdev_add(&ocxlpmem->cdev, ocxlpmem->dev.devt, 1);
> +}
> +
>  /**
>   * ocxlpmem_remove() - Free an OpenCAPI persistent memory device
>   * @pdev: the PCI device information struct
> @@ -376,6 +438,13 @@ static void remove(struct pci_dev *pdev)
>                 if (ocxlpmem->nvdimm_bus)
>                         nvdimm_bus_unregister(ocxlpmem->nvdimm_bus);
>
> +               /*
> +                * Remove the cdev early to prevent a race against userspace
> +                * via the char dev
> +                */
> +               if (ocxlpmem->cdev.owner)
> +                       cdev_del(&ocxlpmem->cdev);
> +
>                 device_unregister(&ocxlpmem->dev);
>         }
>  }
> @@ -527,11 +596,18 @@ static int probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>                 goto err;
>         }
>
> -       if (setup_command_metadata(ocxlpmem)) {
> +       rc = setup_command_metadata(ocxlpmem);
> +       if (rc) {
>                 dev_err(&pdev->dev, "Could not read command metadata\n");
>                 goto err;
>         }
>
> +       rc = create_cdev(ocxlpmem);
> +       if (rc) {
> +               dev_err(&pdev->dev, "Could not create character device\n");
> +               goto err;
> +       }
> +
>         elapsed = 0;
>         timeout = ocxlpmem->readiness_timeout +
>                   ocxlpmem->memory_available_timeout;
> @@ -599,6 +675,36 @@ static struct pci_driver pci_driver = {
>         .shutdown = remove,
>  };
>
> +static int file_init(void)
> +{
> +       int rc;
> +
> +       rc = alloc_chrdev_region(&ocxlpmem_dev, 0, NUM_MINORS, "ocxlpmem");
> +       if (rc) {
> +               idr_destroy(&minors_idr);
> +               pr_err("Unable to allocate OpenCAPI persistent memory major number: %d\n",
> +                      rc);
> +               return rc;
> +       }
> +
> +       ocxlpmem_class = class_create(THIS_MODULE, "ocxlpmem");
> +       if (IS_ERR(ocxlpmem_class)) {
> +               idr_destroy(&minors_idr);
> +               pr_err("Unable to create ocxlpmem class\n");
> +               unregister_chrdev_region(ocxlpmem_dev, NUM_MINORS);
> +               return PTR_ERR(ocxlpmem_class);
> +       }
> +
> +       return 0;
> +}
> +
> +static void file_exit(void)
> +{
> +       class_destroy(ocxlpmem_class);
> +       unregister_chrdev_region(ocxlpmem_dev, NUM_MINORS);
> +       idr_destroy(&minors_idr);
> +}
> +
>  static int __init ocxlpmem_init(void)
>  {
>         int rc;
> @@ -606,16 +712,23 @@ static int __init ocxlpmem_init(void)
>         mutex_init(&minors_idr_lock);
>         idr_init(&minors_idr);
>
> -       rc = pci_register_driver(&pci_driver);
> +       rc = file_init();
>         if (rc)
>                 return rc;
>
> +       rc = pci_register_driver(&pci_driver);
> +       if (rc) {
> +               file_exit();
> +               return rc;
> +       }
> +
>         return 0;
>  }
>
>  static void ocxlpmem_exit(void)
>  {
>         pci_unregister_driver(&pci_driver);
> +       file_exit();
>  }
>
>  module_init(ocxlpmem_init);
> diff --git a/drivers/nvdimm/ocxl/ocxlpmem.h b/drivers/nvdimm/ocxl/ocxlpmem.h
> index b72b3f909fc3..ee3bd651f254 100644
> --- a/drivers/nvdimm/ocxl/ocxlpmem.h
> +++ b/drivers/nvdimm/ocxl/ocxlpmem.h
> @@ -2,6 +2,7 @@
>  // Copyright 2020 IBM Corp.
>
>  #include <linux/pci.h>
> +#include <linux/cdev.h>
>  #include <misc/ocxl.h>
>  #include <linux/libnvdimm.h>
>  #include <linux/mm.h>
> @@ -103,6 +104,7 @@ struct command_metadata {
>  struct ocxlpmem {
>         struct device dev;
>         struct pci_dev *pdev;
> +       struct cdev cdev;
>         struct ocxl_fn *ocxl_fn;
>         struct nd_interleave_set nd_set;
>         struct nvdimm_bus_descriptor bus_desc;
> --
> 2.24.1
>

^ permalink raw reply

* Re: [PATCH v4 13/25] nvdimm/ocxl: Read the capability registers & wait for device ready
From: Dan Williams @ 2020-04-02  0:20 UTC (permalink / raw)
  To: Alastair D'Silva
  Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
	Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
	Rob Herring, Dave Jiang, linux-nvdimm, Aneesh Kumar K . V,
	Krzysztof Kozlowski, Anju T Sudhakar, Mahesh Salgaonkar,
	Andrew Donnellan, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
	Cédric Le Goater, Thomas Gleixner, Hari Bathini, Linux MM,
	Greg Kroah-Hartman, Linux Kernel Mailing List, Vishal Verma,
	Frederic Barrat, Paul Mackerras, Andrew Morton, linuxppc-dev,
	David S. Miller
In-Reply-To: <20200327071202.2159885-14-alastair@d-silva.org>

On Sun, Mar 29, 2020 at 10:23 PM Alastair D'Silva <alastair@d-silva.org> wrote:
>
> This patch reads timeouts & firmware version from the controller, and
> uses those timeouts to wait for the controller to report that it is ready
> before handing the memory over to libnvdimm.
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
>  drivers/nvdimm/ocxl/Makefile            |  2 +-
>  drivers/nvdimm/ocxl/main.c              | 85 +++++++++++++++++++++++++
>  drivers/nvdimm/ocxl/ocxlpmem.h          | 29 +++++++++
>  drivers/nvdimm/ocxl/ocxlpmem_internal.c | 19 ++++++
>  4 files changed, 134 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/nvdimm/ocxl/ocxlpmem_internal.c
>
> diff --git a/drivers/nvdimm/ocxl/Makefile b/drivers/nvdimm/ocxl/Makefile
> index e0e8ade1987a..bab97082e062 100644
> --- a/drivers/nvdimm/ocxl/Makefile
> +++ b/drivers/nvdimm/ocxl/Makefile
> @@ -4,4 +4,4 @@ ccflags-$(CONFIG_PPC_WERROR)    += -Werror
>
>  obj-$(CONFIG_OCXL_PMEM) += ocxlpmem.o
>
> -ocxlpmem-y := main.o
> \ No newline at end of file
> +ocxlpmem-y := main.o ocxlpmem_internal.o
> diff --git a/drivers/nvdimm/ocxl/main.c b/drivers/nvdimm/ocxl/main.c
> index c0066fedf9cc..be76acd33d74 100644
> --- a/drivers/nvdimm/ocxl/main.c
> +++ b/drivers/nvdimm/ocxl/main.c
> @@ -8,6 +8,7 @@
>
>  #include <linux/module.h>
>  #include <misc/ocxl.h>
> +#include <linux/delay.h>
>  #include <linux/ndctl.h>
>  #include <linux/mm_types.h>
>  #include <linux/memory_hotplug.h>
> @@ -327,6 +328,50 @@ static void remove(struct pci_dev *pdev)
>         }
>  }
>
> +/**
> + * read_device_metadata() - Retrieve config information from the AFU and save it for future use
> + * @ocxlpmem: the device metadata
> + * Return: 0 on success, negative on failure
> + */
> +static int read_device_metadata(struct ocxlpmem *ocxlpmem)
> +{
> +       u64 val;
> +       int rc;
> +
> +       rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_CCAP0,
> +                                    OCXL_LITTLE_ENDIAN, &val);

This calling convention would seem to defeat the ability of sparse to
validate endian correctness. That's independent of this series, but I
wonder how does someone review why this argument is sometimes
OCXL_LITTLE_ENDIAN and sometimes OCXL_HOST_ENDIAN?

> +       if (rc)
> +               return rc;
> +
> +       ocxlpmem->scm_revision = val & 0xFFFF;
> +       ocxlpmem->read_latency = (val >> 32) & 0xFFFF;
> +       ocxlpmem->readiness_timeout = (val >> 48) & 0x0F;
> +       ocxlpmem->memory_available_timeout = val >> 52;

Maybe some macros to parse out these register fields?

> +
> +       rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_CCAP1,
> +                                    OCXL_LITTLE_ENDIAN, &val);
> +       if (rc)
> +               return rc;
> +
> +       ocxlpmem->max_controller_dump_size = val & 0xFFFFFFFF;
> +
> +       // Extract firmware version text
> +       rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_FWVER,
> +                                    OCXL_HOST_ENDIAN,
> +                                    (u64 *)ocxlpmem->fw_version);
> +       if (rc)
> +               return rc;
> +
> +       ocxlpmem->fw_version[8] = '\0';
> +
> +       dev_info(&ocxlpmem->dev,
> +                "Firmware version '%s' SCM revision %d:%d\n",
> +                ocxlpmem->fw_version, ocxlpmem->scm_revision >> 4,
> +                ocxlpmem->scm_revision & 0x0F);

Does the driver need to be chatty here. If this data is relevant
should it appear in sysfs by default?

> +
> +       return 0;
> +}
> +
>  /**
>   * probe_function0() - Set up function 0 for an OpenCAPI persistent memory device
>   * This is important as it enables templates higher than 0 across all other
> @@ -359,6 +404,9 @@ static int probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  {
>         struct ocxlpmem *ocxlpmem;
>         int rc;
> +       u64 chi;
> +       u16 elapsed, timeout;
> +       bool ready = false;
>
>         if (PCI_FUNC(pdev->devfn) == 0)
>                 return probe_function0(pdev);
> @@ -413,6 +461,43 @@ static int probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>                 goto err;
>         }
>
> +       rc = read_device_metadata(ocxlpmem);
> +       if (rc) {
> +               dev_err(&pdev->dev, "Could not read metadata\n");
> +               goto err;
> +       }
> +
> +       elapsed = 0;
> +       timeout = ocxlpmem->readiness_timeout +
> +                 ocxlpmem->memory_available_timeout;
> +
> +       while (true) {
> +               rc = ocxlpmem_chi(ocxlpmem, &chi);
> +               ready = (chi & (GLOBAL_MMIO_CHI_CRDY | GLOBAL_MMIO_CHI_MA)) ==
> +                       (GLOBAL_MMIO_CHI_CRDY | GLOBAL_MMIO_CHI_MA);
> +
> +               if (ready)
> +                       break;
> +
> +               if (elapsed++ > timeout) {
> +                       dev_err(&ocxlpmem->dev,
> +                               "OpenCAPI Persistent Memory ready timeout.\n");
> +
> +                       if (!(chi & GLOBAL_MMIO_CHI_CRDY))
> +                               dev_err(&ocxlpmem->dev,
> +                                       "controller is not ready.\n");
> +
> +                       if (!(chi & GLOBAL_MMIO_CHI_MA))
> +                               dev_err(&ocxlpmem->dev,
> +                                       "controller does not have memory available.\n");
> +
> +                       rc = -ENXIO;
> +                       goto err;
> +               }
> +
> +               msleep(1000);

At platform boot this is going to serialize / delay other pci hardware
init. Do you need this determination to be synchronous with the call
to ->probe()? If not, let's move it out of line. For example nvdimm
device registration is asynchronous by default with options to flush
if userspace needs to know that the kernel has finished loading
drivers.

> +       }
> +
>         rc = register_lpc_mem(ocxlpmem);
>         if (rc) {
>                 dev_err(&pdev->dev,
> diff --git a/drivers/nvdimm/ocxl/ocxlpmem.h b/drivers/nvdimm/ocxl/ocxlpmem.h
> index 322387873b4b..3eadbe19f6d0 100644
> --- a/drivers/nvdimm/ocxl/ocxlpmem.h
> +++ b/drivers/nvdimm/ocxl/ocxlpmem.h
> @@ -93,4 +93,33 @@ struct ocxlpmem {
>         void *metadata_addr;
>         struct resource pmem_res;
>         struct nd_region *nd_region;
> +       char fw_version[8 + 1];
> +
> +       u32 max_controller_dump_size;
> +       u16 scm_revision; // major/minor
> +       u8 readiness_timeout;  /* The worst case time (in seconds) that the host
> +                               * shall wait for the controller to become
> +                               * operational following a reset (CHI.CRDY).
> +                               */
> +       u8 memory_available_timeout;  /* The worst case time (in seconds) that
> +                                      * the host shall wait for memory to
> +                                      * become available following a reset
> +                                      * (CHI.MA).
> +                                      */
> +
> +       u16 read_latency; /* The nominal measure of latency (in nanoseconds)
> +                          * associated with an unassisted read of a memory
> +                          * block.
> +                          * This represents the capability of the raw media
> +                          * technology without assistance
> +                          */
>  };
> +
> +/**
> + * ocxlpmem_chi() - Get the value of the CHI register
> + * @ocxlpmem: the device metadata
> + * @chi: returns the CHI value
> + *
> + * Returns 0 on success, negative on error
> + */
> +int ocxlpmem_chi(const struct ocxlpmem *ocxlpmem, u64 *chi);
> diff --git a/drivers/nvdimm/ocxl/ocxlpmem_internal.c b/drivers/nvdimm/ocxl/ocxlpmem_internal.c
> new file mode 100644
> index 000000000000..5578169b7515
> --- /dev/null
> +++ b/drivers/nvdimm/ocxl/ocxlpmem_internal.c
> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright 2020 IBM Corp.
> +
> +#include <misc/ocxl.h>
> +#include <linux/delay.h>
> +#include "ocxlpmem.h"
> +
> +int ocxlpmem_chi(const struct ocxlpmem *ocxlpmem, u64 *chi)
> +{
> +       u64 val;
> +       int rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_CHI,
> +                                        OCXL_LITTLE_ENDIAN, &val);
> +       if (rc)
> +               return rc;
> +
> +       *chi = val;
> +
> +       return 0;
> +}
> --
> 2.24.1
>

^ permalink raw reply

* Re: [PATCH v3 1/1] ppc/crash: Reset spinlocks during crash
From: Leonardo Bras @ 2020-04-01 23:53 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Greg Kroah-Hartman, linuxppc-dev, linux-kernel, Alexios Zavras,
	Paul Mackerras, Thomas Gleixner, Enrico Weigelt
In-Reply-To: <20200401092618.GW20713@hirez.programming.kicks-ass.net>

[-- Attachment #1: Type: text/plain, Size: 524 bytes --]

Hello Peter, 

On Wed, 2020-04-01 at 11:26 +0200, Peter Zijlstra wrote:
> You might want to add a note to your asm/spinlock.h that you rely on
> spin_unlock() unconditionally clearing a lock.
> 
> This isn't naturally true for all lock implementations. Consider ticket
> locks, doing a surplus unlock will wreck your lock state in that case.
> So anybody poking at the powerpc spinlock implementation had better know
> you rely on this.

Good idea. I will add this to my changes and generate a v4.

Thank you,

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v4 03/16] powerpc: Use a datatype for instructions
From: Jordan Niethe @ 2020-04-01 23:52 UTC (permalink / raw)
  To: Balamuruhan S
  Cc: Alistair Popple, Nicholas Piggin, linuxppc-dev, Daniel Axtens
In-Reply-To: <1b00d41261edb8ce259978ff57c8c7d174fc7834.camel@linux.ibm.com>

On Wed, Apr 1, 2020 at 9:32 PM Balamuruhan S <bala24@linux.ibm.com> wrote:
>
> On Fri, 2020-03-20 at 16:17 +1100, Jordan Niethe wrote:
> > Currently unsigned ints are used to represent instructions on powerpc.
> > This has worked well as instructions have always been 4 byte words.
> > However, a future ISA version will introduce some changes to
> > instructions that mean this scheme will no longer work as well. This
> > change is Prefixed Instructions. A prefixed instruction is made up of a
> > word prefix followed by a word suffix to make an 8 byte double word
> > instruction. No matter the endianess of the system the prefix always
> > comes first. Prefixed instructions are only planned for powerpc64.
> >
> > Introduce a ppc_inst type to represent both prefixed and word
> > instructions on powerpc64 while keeping it possible to exclusively have
> > word instructions on powerpc32, A latter patch will expand the type to
> > include prefixed instructions but for now just typedef it to a u32.
> >
> > Later patches will introduce helper functions and macros for
> > manipulating the instructions so that powerpc64 and powerpc32 might
> > maintain separate type definitions.
> >
> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > ---
> >  arch/powerpc/include/asm/code-patching.h | 31 +++++------
> >  arch/powerpc/include/asm/inst.h          | 53 +++++++++++++++++++
> >  arch/powerpc/include/asm/sstep.h         |  5 +-
> >  arch/powerpc/kernel/align.c              |  2 +-
> >  arch/powerpc/kernel/hw_breakpoint.c      |  3 +-
> >  arch/powerpc/kernel/kprobes.c            |  2 +-
> >  arch/powerpc/kernel/mce_power.c          |  5 +-
> >  arch/powerpc/kernel/optprobes.c          | 10 ++--
> >  arch/powerpc/kernel/trace/ftrace.c       | 66 ++++++++++++------------
> >  arch/powerpc/kvm/emulate_loadstore.c     |  1 +
> >  arch/powerpc/lib/code-patching.c         | 54 +++++++++----------
> >  arch/powerpc/lib/sstep.c                 |  4 +-
> >  arch/powerpc/lib/test_emulate_step.c     |  9 ++--
> >  arch/powerpc/xmon/xmon.c                 | 12 ++---
> >  14 files changed, 160 insertions(+), 97 deletions(-)
> >  create mode 100644 arch/powerpc/include/asm/inst.h
> >
> > diff --git a/arch/powerpc/include/asm/code-patching.h
> > b/arch/powerpc/include/asm/code-patching.h
> > index 898b54262881..cb5106f92d67 100644
> > --- a/arch/powerpc/include/asm/code-patching.h
> > +++ b/arch/powerpc/include/asm/code-patching.h
> > @@ -11,6 +11,7 @@
> >  #include <linux/string.h>
> >  #include <linux/kallsyms.h>
> >  #include <asm/asm-compat.h>
> > +#include <asm/inst.h>
> >
> >  /* Flags for create_branch:
> >   * "b"   == create_branch(addr, target, 0);
> > @@ -22,27 +23,27 @@
> >  #define BRANCH_ABSOLUTE      0x2
> >
> >  bool is_offset_in_branch_range(long offset);
> > -unsigned int create_branch(const unsigned int *addr,
> > +ppc_inst create_branch(const ppc_inst *addr,
> >                          unsigned long target, int flags);
> > -unsigned int create_cond_branch(const unsigned int *addr,
> > +unsigned int create_cond_branch(const ppc_inst *addr,
> >                               unsigned long target, int flags);
> > -int patch_branch(unsigned int *addr, unsigned long target, int flags);
> > -int patch_instruction(unsigned int *addr, unsigned int instr);
> > -int raw_patch_instruction(unsigned int *addr, unsigned int instr);
> > +int patch_branch(ppc_inst *addr, unsigned long target, int flags);
> > +int patch_instruction(ppc_inst *addr, ppc_inst instr);
>
> we need to handle this change for its user in epapr_paravirt.c,
Thanks, good catch.
>
> arch/powerpc/kernel/epapr_paravirt.c: In function 'early_init_dt_scan_epapr':
> arch/powerpc/kernel/epapr_paravirt.c:40:48: error: incompatible type for
> argument 2 of 'patch_instruction'
>    40 |   patch_instruction(epapr_hypercall_start + i, inst);
>       |                                                ^~~~
>       |                                                |
>       |                                                u32 {aka unsigned int}
> In file included from arch/powerpc/kernel/epapr_paravirt.c:12:
> ./arch/powerpc/include/asm/code-patching.h:31:44: note: expected 'ppc_inst'
> {aka 'struct ppc_inst'} but argument is of type 'u32' {aka 'unsigned int'}
>    31 | int patch_instruction(void *addr, ppc_inst instr);
>       |                                   ~~~~~~~~~^~~~~
> make[2]: *** [scripts/Makefile.build:268: arch/powerpc/kernel/epapr_paravirt.o]
> Error 1
> make[1]: *** [scripts/Makefile.build:505: arch/powerpc/kernel] Error 2
> make: *** [Makefile:1683: arch/powerpc] Error 2
>
>
> -- Bala
>
> > +int raw_patch_instruction(ppc_inst *addr, ppc_inst instr);
> >
> >  static inline unsigned long patch_site_addr(s32 *site)
> >  {
> >       return (unsigned long)site + *site;
> >  }
> >
> > -static inline int patch_instruction_site(s32 *site, unsigned int instr)
> > +static inline int patch_instruction_site(s32 *site, ppc_inst instr)
> >  {
> > -     return patch_instruction((unsigned int *)patch_site_addr(site), instr);
> > +     return patch_instruction((ppc_inst *)patch_site_addr(site), instr);
> >  }
> >
> >  static inline int patch_branch_site(s32 *site, unsigned long target, int
> > flags)
> >  {
> > -     return patch_branch((unsigned int *)patch_site_addr(site), target,
> > flags);
> > +     return patch_branch((ppc_inst *)patch_site_addr(site), target, flags);
> >  }
> >
> >  static inline int modify_instruction(unsigned int *addr, unsigned int clr,
> > @@ -56,13 +57,13 @@ static inline int modify_instruction_site(s32 *site,
> > unsigned int clr, unsigned
> >       return modify_instruction((unsigned int *)patch_site_addr(site), clr,
> > set);
> >  }
> >
> > -int instr_is_relative_branch(unsigned int instr);
> > -int instr_is_relative_link_branch(unsigned int instr);
> > -int instr_is_branch_to_addr(const unsigned int *instr, unsigned long addr);
> > -unsigned long branch_target(const unsigned int *instr);
> > -unsigned int translate_branch(const unsigned int *dest,
> > -                           const unsigned int *src);
> > -extern bool is_conditional_branch(unsigned int instr);
> > +int instr_is_relative_branch(ppc_inst instr);
> > +int instr_is_relative_link_branch(ppc_inst instr);
> > +int instr_is_branch_to_addr(const ppc_inst *instr, unsigned long addr);
> > +unsigned long branch_target(const ppc_inst *instr);
> > +ppc_inst translate_branch(const ppc_inst *dest,
> > +                           const ppc_inst *src);
> > +extern bool is_conditional_branch(ppc_inst instr);
> >  #ifdef CONFIG_PPC_BOOK3E_64
> >  void __patch_exception(int exc, unsigned long addr);
> >  #define patch_exception(exc, name) do { \
> > diff --git a/arch/powerpc/include/asm/inst.h
> > b/arch/powerpc/include/asm/inst.h
> > new file mode 100644
> > index 000000000000..7c8596ee411e
> > --- /dev/null
> > +++ b/arch/powerpc/include/asm/inst.h
> > @@ -0,0 +1,53 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +#ifndef _ASM_INST_H
> > +#define _ASM_INST_H
> > +
> > +/*
> > + * Instruction data type for POWER
> > + */
> > +
> > +typedef u32 ppc_inst;
> > +
> > +#define PPC_INST(x) (x)
> > +
> > +static inline int ppc_inst_len(ppc_inst x)
> > +{
> > +     return sizeof(ppc_inst);
> > +}
> > +
> > +static inline int ppc_inst_opcode(ppc_inst x)
> > +{
> > +     return x >> 26;
> > +}
> > +
> > +static inline u32 ppc_inst_word(ppc_inst x)
> > +{
> > +     return x;
> > +}
> > +
> > +static inline ppc_inst ppc_inst_read(const ppc_inst *ptr)
> > +{
> > +     return *(ppc_inst *)ptr;
> > +}
> > +
> > +static inline void ppc_inst_write(void *ptr, ppc_inst x)
> > +{
> > +     *(ppc_inst *)ptr = x;
> > +}
> > +
> > +static inline bool ppc_inst_equal(ppc_inst x, ppc_inst y)
> > +{
> > +     return x == y;
> > +}
> > +
> > +static inline bool ppc_inst_null(ppc_inst x)
> > +{
> > +     return x == 0;
> > +}
> > +
> > +static inline u32 ppc_inst_mask(ppc_inst x, u32 mask)
> > +{
> > +     return ppc_inst_word(x) & mask;
> > +}
> > +
> > +#endif /* _ASM_INST_H */
> > diff --git a/arch/powerpc/include/asm/sstep.h
> > b/arch/powerpc/include/asm/sstep.h
> > index 769f055509c9..9353916fcba7 100644
> > --- a/arch/powerpc/include/asm/sstep.h
> > +++ b/arch/powerpc/include/asm/sstep.h
> > @@ -2,6 +2,7 @@
> >  /*
> >   * Copyright (C) 2004 Paul Mackerras <paulus@au.ibm.com>, IBM
> >   */
> > +#include <asm/inst.h>
> >
> >  struct pt_regs;
> >
> > @@ -132,7 +133,7 @@ union vsx_reg {
> >   * otherwise.
> >   */
> >  extern int analyse_instr(struct instruction_op *op, const struct pt_regs
> > *regs,
> > -                      unsigned int instr);
> > +                      ppc_inst instr);
> >
> >  /*
> >   * Emulate an instruction that can be executed just by updating
> > @@ -149,7 +150,7 @@ void emulate_update_regs(struct pt_regs *reg, struct
> > instruction_op *op);
> >   * 0 if it could not be emulated, or -1 for an instruction that
> >   * should not be emulated (rfid, mtmsrd clearing MSR_RI, etc.).
> >   */
> > -extern int emulate_step(struct pt_regs *regs, unsigned int instr);
> > +extern int emulate_step(struct pt_regs *regs, ppc_inst instr);
> >
> >  /*
> >   * Emulate a load or store instruction by reading/writing the
> > diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
> > index 92045ed64976..34594aaa44de 100644
> > --- a/arch/powerpc/kernel/align.c
> > +++ b/arch/powerpc/kernel/align.c
> > @@ -293,7 +293,7 @@ static int emulate_spe(struct pt_regs *regs, unsigned int
> > reg,
> >
> >  int fix_alignment(struct pt_regs *regs)
> >  {
> > -     unsigned int instr;
> > +     ppc_inst instr;
> >       struct instruction_op op;
> >       int r, type;
> >
> > diff --git a/arch/powerpc/kernel/hw_breakpoint.c
> > b/arch/powerpc/kernel/hw_breakpoint.c
> > index 2462cd7c565c..06b97353d231 100644
> > --- a/arch/powerpc/kernel/hw_breakpoint.c
> > +++ b/arch/powerpc/kernel/hw_breakpoint.c
> > @@ -24,6 +24,7 @@
> >  #include <asm/debug.h>
> >  #include <asm/debugfs.h>
> >  #include <asm/hvcall.h>
> > +#include <asm/inst.h>
> >  #include <linux/uaccess.h>
> >
> >  /*
> > @@ -243,7 +244,7 @@ dar_range_overlaps(unsigned long dar, int size, struct
> > arch_hw_breakpoint *info)
> >  static bool stepping_handler(struct pt_regs *regs, struct perf_event *bp,
> >                            struct arch_hw_breakpoint *info)
> >  {
> > -     unsigned int instr = 0;
> > +     ppc_inst instr = 0;
> >       int ret, type, size;
> >       struct instruction_op op;
> >       unsigned long addr = info->address;
> > diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> > index 337516df17d4..e7205adc9820 100644
> > --- a/arch/powerpc/kernel/kprobes.c
> > +++ b/arch/powerpc/kernel/kprobes.c
> > @@ -225,7 +225,7 @@ NOKPROBE_SYMBOL(arch_prepare_kretprobe);
> >  static int try_to_emulate(struct kprobe *p, struct pt_regs *regs)
> >  {
> >       int ret;
> > -     unsigned int insn = *p->ainsn.insn;
> > +     ppc_inst insn = *p->ainsn.insn;
> >
> >       /* regs->nip is also adjusted if emulate_step returns 1 */
> >       ret = emulate_step(regs, insn);
> > diff --git a/arch/powerpc/kernel/mce_power.c
> > b/arch/powerpc/kernel/mce_power.c
> > index 1cbf7f1a4e3d..e65616bb3a3e 100644
> > --- a/arch/powerpc/kernel/mce_power.c
> > +++ b/arch/powerpc/kernel/mce_power.c
> > @@ -20,6 +20,7 @@
> >  #include <asm/sstep.h>
> >  #include <asm/exception-64s.h>
> >  #include <asm/extable.h>
> > +#include <asm/inst.h>
> >
> >  /*
> >   * Convert an address related to an mm to a PFN. NOTE: we are in real
> > @@ -365,7 +366,7 @@ static int mce_find_instr_ea_and_phys(struct pt_regs
> > *regs, uint64_t *addr,
> >        * in real-mode is tricky and can lead to recursive
> >        * faults
> >        */
> > -     int instr;
> > +     ppc_inst instr;
> >       unsigned long pfn, instr_addr;
> >       struct instruction_op op;
> >       struct pt_regs tmp = *regs;
> > @@ -373,7 +374,7 @@ static int mce_find_instr_ea_and_phys(struct pt_regs
> > *regs, uint64_t *addr,
> >       pfn = addr_to_pfn(regs, regs->nip);
> >       if (pfn != ULONG_MAX) {
> >               instr_addr = (pfn << PAGE_SHIFT) + (regs->nip & ~PAGE_MASK);
> > -             instr = *(unsigned int *)(instr_addr);
> > +             instr = *(ppc_inst *)(instr_addr);
> >               if (!analyse_instr(&op, &tmp, instr)) {
> >                       pfn = addr_to_pfn(regs, op.ea);
> >                       *addr = op.ea;
> > diff --git a/arch/powerpc/kernel/optprobes.c
> > b/arch/powerpc/kernel/optprobes.c
> > index 024f7aad1952..f5e8cce438a3 100644
> > --- a/arch/powerpc/kernel/optprobes.c
> > +++ b/arch/powerpc/kernel/optprobes.c
> > @@ -189,8 +189,8 @@ void patch_imm64_load_insns(unsigned long val,
> > kprobe_opcode_t *addr)
> >
> >  int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe
> > *p)
> >  {
> > -     kprobe_opcode_t *buff, branch_op_callback, branch_emulate_step;
> > -     kprobe_opcode_t *op_callback_addr, *emulate_step_addr;
> > +     ppc_inst branch_op_callback, branch_emulate_step;
> > +     kprobe_opcode_t *op_callback_addr, *emulate_step_addr, *buff;
> >       long b_offset;
> >       unsigned long nip, size;
> >       int rc, i;
> > @@ -251,11 +251,11 @@ int arch_prepare_optimized_kprobe(struct
> > optimized_kprobe *op, struct kprobe *p)
> >               goto error;
> >       }
> >
> > -     branch_op_callback = create_branch((unsigned int *)buff +
> > TMPL_CALL_HDLR_IDX,
> > +     branch_op_callback = create_branch((ppc_inst *)buff +
> > TMPL_CALL_HDLR_IDX,
> >                               (unsigned long)op_callback_addr,
> >                               BRANCH_SET_LINK);
> >
> > -     branch_emulate_step = create_branch((unsigned int *)buff +
> > TMPL_EMULATE_IDX,
> > +     branch_emulate_step = create_branch((ppc_inst *)buff +
> > TMPL_EMULATE_IDX,
> >                               (unsigned long)emulate_step_addr,
> >                               BRANCH_SET_LINK);
> >
> > @@ -316,7 +316,7 @@ void arch_optimize_kprobes(struct list_head *oplist)
> >               memcpy(op->optinsn.copied_insn, op->kp.addr,
> >                                              RELATIVEJUMP_SIZE);
> >               patch_instruction(op->kp.addr,
> > -                     create_branch((unsigned int *)op->kp.addr,
> > +                     create_branch((ppc_inst *)op->kp.addr,
> >                                     (unsigned long)op->optinsn.insn, 0));
> >               list_del_init(&op->list);
> >       }
> > diff --git a/arch/powerpc/kernel/trace/ftrace.c
> > b/arch/powerpc/kernel/trace/ftrace.c
> > index 7ea0ca044b65..5787ccffb4df 100644
> > --- a/arch/powerpc/kernel/trace/ftrace.c
> > +++ b/arch/powerpc/kernel/trace/ftrace.c
> > @@ -27,6 +27,7 @@
> >  #include <asm/code-patching.h>
> >  #include <asm/ftrace.h>
> >  #include <asm/syscall.h>
> > +#include <asm/inst.h>
> >
> >
> >  #ifdef CONFIG_DYNAMIC_FTRACE
> > @@ -40,23 +41,23 @@
> >  #define      NUM_FTRACE_TRAMPS       8
> >  static unsigned long ftrace_tramps[NUM_FTRACE_TRAMPS];
> >
> > -static unsigned int
> > +static ppc_inst
> >  ftrace_call_replace(unsigned long ip, unsigned long addr, int link)
> >  {
> > -     unsigned int op;
> > +     ppc_inst op;
> >
> >       addr = ppc_function_entry((void *)addr);
> >
> >       /* if (link) set op to 'bl' else 'b' */
> > -     op = create_branch((unsigned int *)ip, addr, link ? 1 : 0);
> > +     op = create_branch((ppc_inst *)ip, addr, link ? 1 : 0);
> >
> >       return op;
> >  }
> >
> >  static int
> > -ftrace_modify_code(unsigned long ip, unsigned int old, unsigned int new)
> > +ftrace_modify_code(unsigned long ip, ppc_inst old, ppc_inst new)
> >  {
> > -     unsigned int replaced;
> > +     ppc_inst replaced;
> >
> >       /*
> >        * Note:
> > @@ -78,7 +79,7 @@ ftrace_modify_code(unsigned long ip, unsigned int old,
> > unsigned int new)
> >       }
> >
> >       /* replace the text with the new text */
> > -     if (patch_instruction((unsigned int *)ip, new))
> > +     if (patch_instruction((ppc_inst *)ip, new))
> >               return -EPERM;
> >
> >       return 0;
> > @@ -87,25 +88,25 @@ ftrace_modify_code(unsigned long ip, unsigned int old,
> > unsigned int new)
> >  /*
> >   * Helper functions that are the same for both PPC64 and PPC32.
> >   */
> > -static int test_24bit_addr(unsigned long ip, unsigned long addr)
> > +static ppc_inst test_24bit_addr(unsigned long ip, unsigned long addr)
> >  {
> >       addr = ppc_function_entry((void *)addr);
> >
> >       /* use the create_branch to verify that this offset can be branched */
> > -     return create_branch((unsigned int *)ip, addr, 0);
> > +     return create_branch((ppc_inst *)ip, addr, 0);
> >  }
> >
> > -static int is_bl_op(unsigned int op)
> > +static int is_bl_op(ppc_inst op)
> >  {
> >       return (op & 0xfc000003) == 0x48000001;
> >  }
> >
> > -static int is_b_op(unsigned int op)
> > +static int is_b_op(ppc_inst op)
> >  {
> >       return (op & 0xfc000003) == 0x48000000;
> >  }
> >
> > -static unsigned long find_bl_target(unsigned long ip, unsigned int op)
> > +static unsigned long find_bl_target(unsigned long ip, ppc_inst op)
> >  {
> >       int offset;
> >
> > @@ -125,7 +126,7 @@ __ftrace_make_nop(struct module *mod,
> >  {
> >       unsigned long entry, ptr, tramp;
> >       unsigned long ip = rec->ip;
> > -     unsigned int op, pop;
> > +     ppc_inst op, pop;
> >
> >       /* read where this goes */
> >       if (probe_kernel_read(&op, (void *)ip, sizeof(int))) {
> > @@ -204,7 +205,7 @@ __ftrace_make_nop(struct module *mod,
> >       }
> >  #endif /* CONFIG_MPROFILE_KERNEL */
> >
> > -     if (patch_instruction((unsigned int *)ip, pop)) {
> > +     if (patch_instruction((ppc_inst *)ip, pop)) {
> >               pr_err("Patching NOP failed.\n");
> >               return -EPERM;
> >       }
> > @@ -217,7 +218,7 @@ static int
> >  __ftrace_make_nop(struct module *mod,
> >                 struct dyn_ftrace *rec, unsigned long addr)
> >  {
> > -     unsigned int op;
> > +     ppc_inst op;
> >       unsigned int jmp[4];
> >       unsigned long ip = rec->ip;
> >       unsigned long tramp;
> > @@ -276,7 +277,7 @@ __ftrace_make_nop(struct module *mod,
> >
> >       op = PPC_INST_NOP;
> >
> > -     if (patch_instruction((unsigned int *)ip, op))
> > +     if (patch_instruction((ppc_inst *)ip, op))
> >               return -EPERM;
> >
> >       return 0;
> > @@ -322,7 +323,8 @@ static int add_ftrace_tramp(unsigned long tramp)
> >   */
> >  static int setup_mcount_compiler_tramp(unsigned long tramp)
> >  {
> > -     int i, op;
> > +     int i;
> > +     ppc_inst op;
> >       unsigned long ptr;
> >       static unsigned long ftrace_plt_tramps[NUM_FTRACE_TRAMPS];
> >
> > @@ -388,7 +390,7 @@ static int setup_mcount_compiler_tramp(unsigned long
> > tramp)
> >  static int __ftrace_make_nop_kernel(struct dyn_ftrace *rec, unsigned long
> > addr)
> >  {
> >       unsigned long tramp, ip = rec->ip;
> > -     unsigned int op;
> > +     ppc_inst op;
> >
> >       /* Read where this goes */
> >       if (probe_kernel_read(&op, (void *)ip, sizeof(int))) {
> > @@ -416,7 +418,7 @@ static int __ftrace_make_nop_kernel(struct dyn_ftrace
> > *rec, unsigned long addr)
> >               }
> >       }
> >
> > -     if (patch_instruction((unsigned int *)ip, PPC_INST_NOP)) {
> > +     if (patch_instruction((ppc_inst *)ip, PPC_INST_NOP)) {
> >               pr_err("Patching NOP failed.\n");
> >               return -EPERM;
> >       }
> > @@ -428,7 +430,7 @@ int ftrace_make_nop(struct module *mod,
> >                   struct dyn_ftrace *rec, unsigned long addr)
> >  {
> >       unsigned long ip = rec->ip;
> > -     unsigned int old, new;
> > +     ppc_inst old, new;
> >
> >       /*
> >        * If the calling address is more that 24 bits away,
> > @@ -481,7 +483,7 @@ int ftrace_make_nop(struct module *mod,
> >   */
> >  #ifndef CONFIG_MPROFILE_KERNEL
> >  static int
> > -expected_nop_sequence(void *ip, unsigned int op0, unsigned int op1)
> > +expected_nop_sequence(void *ip, ppc_inst op0, ppc_inst op1)
> >  {
> >       /*
> >        * We expect to see:
> > @@ -510,7 +512,7 @@ expected_nop_sequence(void *ip, unsigned int op0,
> > unsigned int op1)
> >  static int
> >  __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> >  {
> > -     unsigned int op[2];
> > +     ppc_inst op[2];
> >       void *ip = (void *)rec->ip;
> >       unsigned long entry, ptr, tramp;
> >       struct module *mod = rec->arch.mod;
> > @@ -574,7 +576,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long
> > addr)
> >  static int
> >  __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> >  {
> > -     unsigned int op;
> > +     ppc_inst op;
> >       unsigned long ip = rec->ip;
> >
> >       /* read where this goes */
> > @@ -594,7 +596,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long
> > addr)
> >       }
> >
> >       /* create the branch to the trampoline */
> > -     op = create_branch((unsigned int *)ip,
> > +     op = create_branch((ppc_inst *)ip,
> >                          rec->arch.mod->arch.tramp, BRANCH_SET_LINK);
> >       if (!op) {
> >               pr_err("REL24 out of range!\n");
> > @@ -613,7 +615,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long
> > addr)
> >
> >  static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned long
> > addr)
> >  {
> > -     unsigned int op;
> > +     ppc_inst op;
> >       void *ip = (void *)rec->ip;
> >       unsigned long tramp, entry, ptr;
> >
> > @@ -661,7 +663,7 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace
> > *rec, unsigned long addr)
> >  int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> >  {
> >       unsigned long ip = rec->ip;
> > -     unsigned int old, new;
> > +     ppc_inst old, new;
> >
> >       /*
> >        * If the calling address is more that 24 bits away,
> > @@ -700,7 +702,7 @@ static int
> >  __ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
> >                                       unsigned long addr)
> >  {
> > -     unsigned int op;
> > +     ppc_inst op;
> >       unsigned long ip = rec->ip;
> >       unsigned long entry, ptr, tramp;
> >       struct module *mod = rec->arch.mod;
> > @@ -748,7 +750,7 @@ __ftrace_modify_call(struct dyn_ftrace *rec, unsigned
> > long old_addr,
> >       /* The new target may be within range */
> >       if (test_24bit_addr(ip, addr)) {
> >               /* within range */
> > -             if (patch_branch((unsigned int *)ip, addr, BRANCH_SET_LINK)) {
> > +             if (patch_branch((ppc_inst *)ip, addr, BRANCH_SET_LINK)) {
> >                       pr_err("REL24 out of range!\n");
> >                       return -EINVAL;
> >               }
> > @@ -776,7 +778,7 @@ __ftrace_modify_call(struct dyn_ftrace *rec, unsigned
> > long old_addr,
> >       }
> >
> >       /* Ensure branch is within 24 bits */
> > -     if (!create_branch((unsigned int *)ip, tramp, BRANCH_SET_LINK)) {
> > +     if (!create_branch((ppc_inst *)ip, tramp, BRANCH_SET_LINK)) {
> >               pr_err("Branch out of range\n");
> >               return -EINVAL;
> >       }
> > @@ -794,7 +796,7 @@ int ftrace_modify_call(struct dyn_ftrace *rec, unsigned
> > long old_addr,
> >                       unsigned long addr)
> >  {
> >       unsigned long ip = rec->ip;
> > -     unsigned int old, new;
> > +     ppc_inst old, new;
> >
> >       /*
> >        * If the calling address is more that 24 bits away,
> > @@ -834,7 +836,7 @@ int ftrace_modify_call(struct dyn_ftrace *rec, unsigned
> > long old_addr,
> >  int ftrace_update_ftrace_func(ftrace_func_t func)
> >  {
> >       unsigned long ip = (unsigned long)(&ftrace_call);
> > -     unsigned int old, new;
> > +     ppc_inst old, new;
> >       int ret;
> >
> >       old = *(unsigned int *)&ftrace_call;
> > @@ -919,7 +921,7 @@ int ftrace_enable_ftrace_graph_caller(void)
> >       unsigned long ip = (unsigned long)(&ftrace_graph_call);
> >       unsigned long addr = (unsigned long)(&ftrace_graph_caller);
> >       unsigned long stub = (unsigned long)(&ftrace_graph_stub);
> > -     unsigned int old, new;
> > +     ppc_inst old, new;
> >
> >       old = ftrace_call_replace(ip, stub, 0);
> >       new = ftrace_call_replace(ip, addr, 0);
> > @@ -932,7 +934,7 @@ int ftrace_disable_ftrace_graph_caller(void)
> >       unsigned long ip = (unsigned long)(&ftrace_graph_call);
> >       unsigned long addr = (unsigned long)(&ftrace_graph_caller);
> >       unsigned long stub = (unsigned long)(&ftrace_graph_stub);
> > -     unsigned int old, new;
> > +     ppc_inst old, new;
> >
> >       old = ftrace_call_replace(ip, addr, 0);
> >       new = ftrace_call_replace(ip, stub, 0);
> > diff --git a/arch/powerpc/kvm/emulate_loadstore.c
> > b/arch/powerpc/kvm/emulate_loadstore.c
> > index 1139bc56e004..1c9bcbfeb924 100644
> > --- a/arch/powerpc/kvm/emulate_loadstore.c
> > +++ b/arch/powerpc/kvm/emulate_loadstore.c
> > @@ -21,6 +21,7 @@
> >  #include <asm/disassemble.h>
> >  #include <asm/ppc-opcode.h>
> >  #include <asm/sstep.h>
> > +#include <asm/inst.h>
> >  #include "timing.h"
> >  #include "trace.h"
> >
> > diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-
> > patching.c
> > index 3345f039a876..8492b9e2b8db 100644
> > --- a/arch/powerpc/lib/code-patching.c
> > +++ b/arch/powerpc/lib/code-patching.c
> > @@ -17,9 +17,10 @@
> >  #include <asm/page.h>
> >  #include <asm/code-patching.h>
> >  #include <asm/setup.h>
> > +#include <asm/inst.h>
> >
> > -static int __patch_instruction(unsigned int *exec_addr, unsigned int instr,
> > -                            unsigned int *patch_addr)
> > +static int __patch_instruction(ppc_inst *exec_addr, ppc_inst instr,
> > +                            ppc_inst *patch_addr)
> >  {
> >       int err = 0;
> >
> > @@ -33,7 +34,7 @@ static int __patch_instruction(unsigned int *exec_addr,
> > unsigned int instr,
> >       return 0;
> >  }
> >
> > -int raw_patch_instruction(unsigned int *addr, unsigned int instr)
> > +int raw_patch_instruction(ppc_inst *addr, ppc_inst instr)
> >  {
> >       return __patch_instruction(addr, instr, addr);
> >  }
> > @@ -136,10 +137,10 @@ static inline int unmap_patch_area(unsigned long addr)
> >       return 0;
> >  }
> >
> > -static int do_patch_instruction(unsigned int *addr, unsigned int instr)
> > +static int do_patch_instruction(ppc_inst *addr, ppc_inst instr)
> >  {
> >       int err;
> > -     unsigned int *patch_addr = NULL;
> > +     ppc_inst *patch_addr = NULL;
> >       unsigned long flags;
> >       unsigned long text_poke_addr;
> >       unsigned long kaddr = (unsigned long)addr;
> > @@ -176,7 +177,7 @@ static int do_patch_instruction(unsigned int *addr,
> > unsigned int instr)
> >  }
> >  #else /* !CONFIG_STRICT_KERNEL_RWX */
> >
> > -static int do_patch_instruction(unsigned int *addr, unsigned int instr)
> > +static int do_patch_instruction(ppc_inst *addr, ppc_inst instr)
> >  {
> >       return raw_patch_instruction(addr, instr);
> >  }
> > @@ -194,7 +195,7 @@ int patch_instruction(unsigned int *addr, unsigned int
> > instr)
> >  }
> >  NOKPROBE_SYMBOL(patch_instruction);
> >
> > -int patch_branch(unsigned int *addr, unsigned long target, int flags)
> > +int patch_branch(ppc_inst *addr, unsigned long target, int flags)
> >  {
> >       return patch_instruction(addr, create_branch(addr, target, flags));
> >  }
> > @@ -225,7 +226,7 @@ bool is_offset_in_branch_range(long offset)
> >   * Helper to check if a given instruction is a conditional branch
> >   * Derived from the conditional checks in analyse_instr()
> >   */
> > -bool is_conditional_branch(unsigned int instr)
> > +bool is_conditional_branch(ppc_inst instr)
> >  {
> >       unsigned int opcode = instr >> 26;
> >
> > @@ -243,10 +244,10 @@ bool is_conditional_branch(unsigned int instr)
> >  }
> >  NOKPROBE_SYMBOL(is_conditional_branch);
> >
> > -unsigned int create_branch(const unsigned int *addr,
> > +ppc_inst create_branch(const ppc_inst *addr,
> >                          unsigned long target, int flags)
> >  {
> > -     unsigned int instruction;
> > +     ppc_inst instruction;
> >       long offset;
> >
> >       offset = target;
> > @@ -266,7 +267,7 @@ unsigned int create_branch(const unsigned int *addr,
> >  unsigned int create_cond_branch(const unsigned int *addr,
> >                               unsigned long target, int flags)
> >  {
> > -     unsigned int instruction;
> > +     ppc_inst instruction;
> >       long offset;
> >
> >       offset = target;
> > @@ -283,22 +284,22 @@ unsigned int create_cond_branch(const unsigned int
> > *addr,
> >       return instruction;
> >  }
> >
> > -static unsigned int branch_opcode(unsigned int instr)
> > +static unsigned int branch_opcode(ppc_inst instr)
> >  {
> >       return (instr >> 26) & 0x3F;
> >  }
> >
> > -static int instr_is_branch_iform(unsigned int instr)
> > +static int instr_is_branch_iform(ppc_inst instr)
> >  {
> >       return branch_opcode(instr) == 18;
> >  }
> >
> > -static int instr_is_branch_bform(unsigned int instr)
> > +static int instr_is_branch_bform(ppc_inst instr)
> >  {
> >       return branch_opcode(instr) == 16;
> >  }
> >
> > -int instr_is_relative_branch(unsigned int instr)
> > +int instr_is_relative_branch(ppc_inst instr)
> >  {
> >       if (instr & BRANCH_ABSOLUTE)
> >               return 0;
> > @@ -306,12 +307,12 @@ int instr_is_relative_branch(unsigned int instr)
> >       return instr_is_branch_iform(instr) || instr_is_branch_bform(instr);
> >  }
> >
> > -int instr_is_relative_link_branch(unsigned int instr)
> > +int instr_is_relative_link_branch(ppc_inst instr)
> >  {
> >       return instr_is_relative_branch(instr) && (instr & BRANCH_SET_LINK);
> >  }
> >
> > -static unsigned long branch_iform_target(const unsigned int *instr)
> > +static unsigned long branch_iform_target(const ppc_inst *instr)
> >  {
> >       signed long imm;
> >
> > @@ -327,7 +328,7 @@ static unsigned long branch_iform_target(const unsigned
> > int *instr)
> >       return (unsigned long)imm;
> >  }
> >
> > -static unsigned long branch_bform_target(const unsigned int *instr)
> > +static unsigned long branch_bform_target(const ppc_inst *instr)
> >  {
> >       signed long imm;
> >
> > @@ -343,7 +344,7 @@ static unsigned long branch_bform_target(const unsigned
> > int *instr)
> >       return (unsigned long)imm;
> >  }
> >
> > -unsigned long branch_target(const unsigned int *instr)
> > +unsigned long branch_target(const ppc_inst *instr)
> >  {
> >       if (instr_is_branch_iform(*instr))
> >               return branch_iform_target(instr);
> > @@ -353,7 +354,7 @@ unsigned long branch_target(const unsigned int *instr)
> >       return 0;
> >  }
> >
> > -int instr_is_branch_to_addr(const unsigned int *instr, unsigned long addr)
> > +int instr_is_branch_to_addr(const ppc_inst *instr, unsigned long addr)
> >  {
> >       if (instr_is_branch_iform(*instr) || instr_is_branch_bform(*instr))
> >               return branch_target(instr) == addr;
> > @@ -361,7 +362,7 @@ int instr_is_branch_to_addr(const unsigned int *instr,
> > unsigned long addr)
> >       return 0;
> >  }
> >
> > -unsigned int translate_branch(const unsigned int *dest, const unsigned int
> > *src)
> > +ppc_inst translate_branch(const ppc_inst *dest, const ppc_inst *src)
> >  {
> >       unsigned long target;
> >
> > @@ -403,7 +404,7 @@ static void __init test_trampoline(void)
> >
> >  static void __init test_branch_iform(void)
> >  {
> > -     unsigned int instr;
> > +     ppc_inst instr;
> >       unsigned long addr;
> >
> >       addr = (unsigned long)&instr;
> > @@ -478,11 +479,11 @@ static void __init test_branch_iform(void)
> >
> >  static void __init test_create_function_call(void)
> >  {
> > -     unsigned int *iptr;
> > +     ppc_inst *iptr;
> >       unsigned long dest;
> >
> >       /* Check we can create a function call */
> > -     iptr = (unsigned int *)ppc_function_entry(test_trampoline);
> > +     iptr = (ppc_inst *)ppc_function_entry(test_trampoline);
> >       dest = ppc_function_entry(test_create_function_call);
> >       patch_instruction(iptr, create_branch(iptr, dest, BRANCH_SET_LINK));
> >       check(instr_is_branch_to_addr(iptr, dest));
> > @@ -491,7 +492,8 @@ static void __init test_create_function_call(void)
> >  static void __init test_branch_bform(void)
> >  {
> >       unsigned long addr;
> > -     unsigned int *iptr, instr, flags;
> > +     ppc_inst *iptr, instr;
> > +     unsigned int flags;
> >
> >       iptr = &instr;
> >       addr = (unsigned long)iptr;
> > @@ -561,7 +563,7 @@ static void __init test_branch_bform(void)
> >  static void __init test_translate_branch(void)
> >  {
> >       unsigned long addr;
> > -     unsigned int *p, *q;
> > +     ppc_inst *p, *q;
> >       void *buf;
> >
> >       buf = vmalloc(PAGE_ALIGN(0x2000000 + 1));
> > diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> > index c077acb983a1..1d9c766a89fe 100644
> > --- a/arch/powerpc/lib/sstep.c
> > +++ b/arch/powerpc/lib/sstep.c
> > @@ -1163,7 +1163,7 @@ static nokprobe_inline int trap_compare(long v1, long
> > v2)
> >   * otherwise.
> >   */
> >  int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
> > -               unsigned int instr)
> > +               ppc_inst instr)
> >  {
> >       unsigned int opcode, ra, rb, rc, rd, spr, u;
> >       unsigned long int imm;
> > @@ -3101,7 +3101,7 @@ NOKPROBE_SYMBOL(emulate_loadstore);
> >   * or -1 if the instruction is one that should not be stepped,
> >   * such as an rfid, or a mtmsrd that would clear MSR_RI.
> >   */
> > -int emulate_step(struct pt_regs *regs, unsigned int instr)
> > +int emulate_step(struct pt_regs *regs, ppc_inst instr)
> >  {
> >       struct instruction_op op;
> >       int r, err, type;
> > diff --git a/arch/powerpc/lib/test_emulate_step.c
> > b/arch/powerpc/lib/test_emulate_step.c
> > index 42347067739c..158efc8a0f53 100644
> > --- a/arch/powerpc/lib/test_emulate_step.c
> > +++ b/arch/powerpc/lib/test_emulate_step.c
> > @@ -460,7 +460,7 @@ struct compute_test {
> >       struct {
> >               char *descr;
> >               unsigned long flags;
> > -             unsigned int instr;
> > +             ppc_inst instr;
> >               struct pt_regs regs;
> >       } subtests[MAX_SUBTESTS + 1];
> >  };
> > @@ -841,7 +841,7 @@ static struct compute_test compute_tests[] = {
> >  };
> >
> >  static int __init emulate_compute_instr(struct pt_regs *regs,
> > -                                     unsigned int instr)
> > +                                     ppc_inst instr)
> >  {
> >       struct instruction_op op;
> >
> > @@ -859,7 +859,7 @@ static int __init emulate_compute_instr(struct pt_regs
> > *regs,
> >  }
> >
> >  static int __init execute_compute_instr(struct pt_regs *regs,
> > -                                     unsigned int instr)
> > +                                     ppc_inst instr)
> >  {
> >       extern int exec_instr(struct pt_regs *regs);
> >       extern s32 patch__exec_instr;
> > @@ -890,7 +890,8 @@ static void __init run_tests_compute(void)
> >       unsigned long flags;
> >       struct compute_test *test;
> >       struct pt_regs *regs, exp, got;
> > -     unsigned int i, j, k, instr;
> > +     unsigned int i, j, k;
> > +     ppc_inst instr;
> >       bool ignore_gpr, ignore_xer, ignore_ccr, passed;
> >
> >       for (i = 0; i < ARRAY_SIZE(compute_tests); i++) {
> > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> > index 7875d1a37770..a0bc442f9557 100644
> > --- a/arch/powerpc/xmon/xmon.c
> > +++ b/arch/powerpc/xmon/xmon.c
> > @@ -892,7 +892,7 @@ static struct bpt *new_breakpoint(unsigned long a)
> >  static void insert_bpts(void)
> >  {
> >       int i;
> > -     unsigned int instr;
> > +     ppc_inst instr;
> >       struct bpt *bp;
> >
> >       bp = bpts;
> > @@ -914,7 +914,7 @@ static void insert_bpts(void)
> >               patch_instruction(bp->instr, instr);
> >               if (bp->enabled & BP_CIABR)
> >                       continue;
> > -             if (patch_instruction((unsigned int *)bp->address,
> > +             if (patch_instruction((ppc_inst *)bp->address,
> >                                                       bpinstr) != 0) {
> >                       printf("Couldn't write instruction at %lx, "
> >                              "disabling breakpoint there\n", bp->address);
> > @@ -943,7 +943,7 @@ static void remove_bpts(void)
> >  {
> >       int i;
> >       struct bpt *bp;
> > -     unsigned instr;
> > +     ppc_inst instr;
> >
> >       bp = bpts;
> >       for (i = 0; i < NBPTS; ++i, ++bp) {
> > @@ -952,7 +952,7 @@ static void remove_bpts(void)
> >               if (mread(bp->address, &instr, 4) == 4
> >                   && instr == bpinstr
> >                   && patch_instruction(
> > -                     (unsigned int *)bp->address, bp->instr[0]) != 0)
> > +                     (ppc_inst *)bp->address, bp->instr[0]) != 0)
> >                       printf("Couldn't remove breakpoint at %lx\n",
> >                              bp->address);
> >       }
> > @@ -1159,7 +1159,7 @@ static int do_step(struct pt_regs *regs)
> >   */
> >  static int do_step(struct pt_regs *regs)
> >  {
> > -     unsigned int instr;
> > +     ppc_inst instr;
> >       int stepped;
> >
> >       force_enable_xmon();
> > @@ -1325,7 +1325,7 @@ csum(void)
> >   */
> >  static long check_bp_loc(unsigned long addr)
> >  {
> > -     unsigned int instr;
> > +     ppc_inst instr;
> >
> >       addr &= ~3;
> >       if (!is_kernel_addr(addr)) {
>

^ permalink raw reply

* Re: linux-next: manual merge of the net-next tree with the powerpc tree
From: Stephen Rothwell @ 2020-04-01 23:09 UTC (permalink / raw)
  To: Michael Ellerman, PowerPC
  Cc: Networking, Linux Kernel Mailing List, Sourabh Jain,
	Linux Next Mailing List, Christian Brauner, David Miller
In-Reply-To: <20200306102158.0b88e0a0@canb.auug.org.au>

[-- Attachment #1: Type: text/plain, Size: 4405 bytes --]

Hi all,

On Fri, 6 Mar 2020 10:21:58 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Today's linux-next merge of the net-next tree got a conflict in:
> 
>   fs/sysfs/group.c
> 
> between commit:
> 
>   9255782f7061 ("sysfs: Wrap __compat_only_sysfs_link_entry_to_kobj function to change the symlink name")
> 
> from the powerpc tree and commit:
> 
>   303a42769c4c ("sysfs: add sysfs_group{s}_change_owner()")
> 
> from the net-next tree.
> 
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging.  You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
> 
> -- 
> Cheers,
> Stephen Rothwell
> 
> diff --cc fs/sysfs/group.c
> index 1e2a096057bc,5afe0e7ff7cd..000000000000
> --- a/fs/sysfs/group.c
> +++ b/fs/sysfs/group.c
> @@@ -478,4 -457,118 +479,118 @@@ int compat_only_sysfs_link_entry_to_kob
>   	kernfs_put(target);
>   	return PTR_ERR_OR_ZERO(link);
>   }
>  -EXPORT_SYMBOL_GPL(__compat_only_sysfs_link_entry_to_kobj);
>  +EXPORT_SYMBOL_GPL(compat_only_sysfs_link_entry_to_kobj);
> + 
> + static int sysfs_group_attrs_change_owner(struct kernfs_node *grp_kn,
> + 					  const struct attribute_group *grp,
> + 					  struct iattr *newattrs)
> + {
> + 	struct kernfs_node *kn;
> + 	int error;
> + 
> + 	if (grp->attrs) {
> + 		struct attribute *const *attr;
> + 
> + 		for (attr = grp->attrs; *attr; attr++) {
> + 			kn = kernfs_find_and_get(grp_kn, (*attr)->name);
> + 			if (!kn)
> + 				return -ENOENT;
> + 
> + 			error = kernfs_setattr(kn, newattrs);
> + 			kernfs_put(kn);
> + 			if (error)
> + 				return error;
> + 		}
> + 	}
> + 
> + 	if (grp->bin_attrs) {
> + 		struct bin_attribute *const *bin_attr;
> + 
> + 		for (bin_attr = grp->bin_attrs; *bin_attr; bin_attr++) {
> + 			kn = kernfs_find_and_get(grp_kn, (*bin_attr)->attr.name);
> + 			if (!kn)
> + 				return -ENOENT;
> + 
> + 			error = kernfs_setattr(kn, newattrs);
> + 			kernfs_put(kn);
> + 			if (error)
> + 				return error;
> + 		}
> + 	}
> + 
> + 	return 0;
> + }
> + 
> + /**
> +  * sysfs_group_change_owner - change owner of an attribute group.
> +  * @kobj:	The kobject containing the group.
> +  * @grp:	The attribute group.
> +  * @kuid:	new owner's kuid
> +  * @kgid:	new owner's kgid
> +  *
> +  * Returns 0 on success or error code on failure.
> +  */
> + int sysfs_group_change_owner(struct kobject *kobj,
> + 			     const struct attribute_group *grp, kuid_t kuid,
> + 			     kgid_t kgid)
> + {
> + 	struct kernfs_node *grp_kn;
> + 	int error;
> + 	struct iattr newattrs = {
> + 		.ia_valid = ATTR_UID | ATTR_GID,
> + 		.ia_uid = kuid,
> + 		.ia_gid = kgid,
> + 	};
> + 
> + 	if (!kobj->state_in_sysfs)
> + 		return -EINVAL;
> + 
> + 	if (grp->name) {
> + 		grp_kn = kernfs_find_and_get(kobj->sd, grp->name);
> + 	} else {
> + 		kernfs_get(kobj->sd);
> + 		grp_kn = kobj->sd;
> + 	}
> + 	if (!grp_kn)
> + 		return -ENOENT;
> + 
> + 	error = kernfs_setattr(grp_kn, &newattrs);
> + 	if (!error)
> + 		error = sysfs_group_attrs_change_owner(grp_kn, grp, &newattrs);
> + 
> + 	kernfs_put(grp_kn);
> + 
> + 	return error;
> + }
> + EXPORT_SYMBOL_GPL(sysfs_group_change_owner);
> + 
> + /**
> +  * sysfs_groups_change_owner - change owner of a set of attribute groups.
> +  * @kobj:	The kobject containing the groups.
> +  * @groups:	The attribute groups.
> +  * @kuid:	new owner's kuid
> +  * @kgid:	new owner's kgid
> +  *
> +  * Returns 0 on success or error code on failure.
> +  */
> + int sysfs_groups_change_owner(struct kobject *kobj,
> + 			      const struct attribute_group **groups,
> + 			      kuid_t kuid, kgid_t kgid)
> + {
> + 	int error = 0, i;
> + 
> + 	if (!kobj->state_in_sysfs)
> + 		return -EINVAL;
> + 
> + 	if (!groups)
> + 		return 0;
> + 
> + 	for (i = 0; groups[i]; i++) {
> + 		error = sysfs_group_change_owner(kobj, groups[i], kuid, kgid);
> + 		if (error)
> + 			break;
> + 	}
> + 
> + 	return error;
> + }
> + EXPORT_SYMBOL_GPL(sysfs_groups_change_owner);

This is now a conflict between the powerpc tree and Linus' tree.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* RE: [PATCH v4 01/25] powerpc/powernv: Add OPAL calls for LPC memory alloc/release
From: Alastair D'Silva @ 2020-04-01 22:51 UTC (permalink / raw)
  To: 'Dan Williams'
  Cc: 'Madhavan Srinivasan', 'Alexey Kardashevskiy',
	'Masahiro Yamada', 'Oliver O'Halloran',
	'Mauro Carvalho Chehab', 'Ira Weiny',
	'Rob Herring', 'Dave Jiang',
	'linux-nvdimm', 'Aneesh Kumar K . V',
	'Krzysztof Kozlowski', 'Anju T Sudhakar',
	'Mahesh Salgaonkar', 'Andrew Donnellan',
	'Arnd Bergmann', 'Greg Kurz',
	'Nicholas Piggin', 'Cédric Le Goater',
	'Thomas Gleixner', 'Hari Bathini',
	'Linux MM', 'Greg Kroah-Hartman',
	'Linux Kernel Mailing List', 'Vishal Verma',
	'Frederic Barrat', 'Paul Mackerras',
	'Andrew Morton', 'linuxppc-dev',
	'David S. Miller'
In-Reply-To: <CAPcyv4hX9RTWKSLB8OcYY6MK-z5u5WWSaYSGa-8oqPbWU7st8w@mail.gmail.com>

> -----Original Message-----
> From: Dan Williams <dan.j.williams@intel.com>
> Sent: Wednesday, 1 April 2020 7:48 PM
> To: Alastair D'Silva <alastair@d-silva.org>
> Cc: Aneesh Kumar K . V <aneesh.kumar@linux.ibm.com>; Oliver O'Halloran
> <oohall@gmail.com>; Benjamin Herrenschmidt
> <benh@kernel.crashing.org>; Paul Mackerras <paulus@samba.org>; Michael
> Ellerman <mpe@ellerman.id.au>; Frederic Barrat <fbarrat@linux.ibm.com>;
> Andrew Donnellan <ajd@linux.ibm.com>; Arnd Bergmann
> <arnd@arndb.de>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>;
> Vishal Verma <vishal.l.verma@intel.com>; Dave Jiang
> <dave.jiang@intel.com>; Ira Weiny <ira.weiny@intel.com>; Andrew Morton
> <akpm@linux-foundation.org>; Mauro Carvalho Chehab
> <mchehab+samsung@kernel.org>; David S. Miller <davem@davemloft.net>;
> Rob Herring <robh@kernel.org>; Anton Blanchard <anton@ozlabs.org>;
> Krzysztof Kozlowski <krzk@kernel.org>; Mahesh Salgaonkar
> <mahesh@linux.vnet.ibm.com>; Madhavan Srinivasan
> <maddy@linux.vnet.ibm.com>; Cédric Le Goater <clg@kaod.org>; Anju T
> Sudhakar <anju@linux.vnet.ibm.com>; Hari Bathini
> <hbathini@linux.ibm.com>; Thomas Gleixner <tglx@linutronix.de>; Greg
> Kurz <groug@kaod.org>; Nicholas Piggin <npiggin@gmail.com>; Masahiro
> Yamada <yamada.masahiro@socionext.com>; Alexey Kardashevskiy
> <aik@ozlabs.ru>; Linux Kernel Mailing List <linux-kernel@vger.kernel.org>;
> linuxppc-dev <linuxppc-dev@lists.ozlabs.org>; linux-nvdimm <linux-
> nvdimm@lists.01.org>; Linux MM <linux-mm@kvack.org>
> Subject: Re: [PATCH v4 01/25] powerpc/powernv: Add OPAL calls for LPC
> memory alloc/release
> 
> On Sun, Mar 29, 2020 at 10:23 PM Alastair D'Silva <alastair@d-silva.org>
> wrote:
> >
> > Add OPAL calls for LPC memory alloc/release
> >
> 
> This seems to be referencing an existing api definition, can you include a
> pointer to the spec in case someone wanted to understand what these
> routines do? I suspect this is not allocating memory in the traditional sense as
> much as it's allocating physical address space for a device to be mapped?
> 

These API calls were introduced in the following skiboot commit:
https://github.com/open-power/skiboot/commit/1a548857ce1f02f43585b326a891eed18a7b43b3

I'll add it to the description.

> 
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > Acked-by: Andrew Donnellan <ajd@linux.ibm.com>
> > Acked-by: Frederic Barrat <fbarrat@linux.ibm.com>
> > ---
> >  arch/powerpc/include/asm/opal-api.h        | 2 ++
> >  arch/powerpc/include/asm/opal.h            | 2 ++
> >  arch/powerpc/platforms/powernv/opal-call.c | 2 ++
> >  3 files changed, 6 insertions(+)
> >
> > diff --git a/arch/powerpc/include/asm/opal-api.h
> > b/arch/powerpc/include/asm/opal-api.h
> > index c1f25a760eb1..9298e603001b 100644
> > --- a/arch/powerpc/include/asm/opal-api.h
> > +++ b/arch/powerpc/include/asm/opal-api.h
> > @@ -208,6 +208,8 @@
> >  #define OPAL_HANDLE_HMI2                       166
> >  #define        OPAL_NX_COPROC_INIT                     167
> >  #define OPAL_XIVE_GET_VP_STATE                 170
> > +#define OPAL_NPU_MEM_ALLOC                     171
> > +#define OPAL_NPU_MEM_RELEASE                   172
> >  #define OPAL_MPIPL_UPDATE                      173
> >  #define OPAL_MPIPL_REGISTER_TAG                        174
> >  #define OPAL_MPIPL_QUERY_TAG                   175
> > diff --git a/arch/powerpc/include/asm/opal.h
> > b/arch/powerpc/include/asm/opal.h index 9986ac34b8e2..301fea46c7ca
> > 100644
> > --- a/arch/powerpc/include/asm/opal.h
> > +++ b/arch/powerpc/include/asm/opal.h
> > @@ -39,6 +39,8 @@ int64_t opal_npu_spa_clear_cache(uint64_t phb_id,
> uint32_t bdfn,
> >                                 uint64_t PE_handle);  int64_t
> > opal_npu_tl_set(uint64_t phb_id, uint32_t bdfn, long cap,
> >                         uint64_t rate_phys, uint32_t size);
> > +int64_t opal_npu_mem_alloc(u64 phb_id, u32 bdfn, u64 size, __be64
> > +*bar); int64_t opal_npu_mem_release(u64 phb_id, u32 bdfn);
> >
> >  int64_t opal_console_write(int64_t term_number, __be64 *length,
> >                            const uint8_t *buffer); diff --git
> > a/arch/powerpc/platforms/powernv/opal-call.c
> > b/arch/powerpc/platforms/powernv/opal-call.c
> > index 5cd0f52d258f..f26e58b72c04 100644
> > --- a/arch/powerpc/platforms/powernv/opal-call.c
> > +++ b/arch/powerpc/platforms/powernv/opal-call.c
> > @@ -287,6 +287,8 @@ OPAL_CALL(opal_pci_set_pbcq_tunnel_bar,
> OPAL_PCI_SET_PBCQ_TUNNEL_BAR);
> >  OPAL_CALL(opal_sensor_read_u64,
> OPAL_SENSOR_READ_U64);
> >  OPAL_CALL(opal_sensor_group_enable,
> OPAL_SENSOR_GROUP_ENABLE);
> >  OPAL_CALL(opal_nx_coproc_init,                 OPAL_NX_COPROC_INIT);
> > +OPAL_CALL(opal_npu_mem_alloc,                  OPAL_NPU_MEM_ALLOC);
> > +OPAL_CALL(opal_npu_mem_release,
> OPAL_NPU_MEM_RELEASE);
> >  OPAL_CALL(opal_mpipl_update,                   OPAL_MPIPL_UPDATE);
> >  OPAL_CALL(opal_mpipl_register_tag,
> OPAL_MPIPL_REGISTER_TAG);
> >  OPAL_CALL(opal_mpipl_query_tag,
> OPAL_MPIPL_QUERY_TAG);
> > --
> > 2.24.1
> >
> 
> 
> --
> This email has been checked for viruses by AVG.
> https://www.avg.com


-- 
Alastair D'Silva           mob: 0423 762 819
skype: alastair_dsilva     msn: alastair@d-silva.org
blog: http://alastair.d-silva.org    Twitter: @EvilDeece


^ permalink raw reply

* RE: [PATCH v4 00/25] Add support for OpenCAPI Persistent Memory devices
From: Alastair D'Silva @ 2020-04-01 22:44 UTC (permalink / raw)
  To: 'Dan Williams'
  Cc: 'Madhavan Srinivasan', 'Alexey Kardashevskiy',
	'Masahiro Yamada', 'Oliver O'Halloran',
	'Mauro Carvalho Chehab', 'Ira Weiny',
	'Rob Herring', 'Dave Jiang',
	'linux-nvdimm', 'Aneesh Kumar K . V',
	'Krzysztof Kozlowski', 'Anju T Sudhakar',
	'Mahesh Salgaonkar', 'Andrew Donnellan',
	'Arnd Bergmann', 'Greg Kurz',
	'Nicholas Piggin', 'Cédric Le Goater',
	'Thomas Gleixner', 'Hari Bathini',
	'Linux MM', 'Greg Kroah-Hartman',
	'Linux Kernel Mailing List', 'Vishal Verma',
	'Frederic Barrat', 'Paul Mackerras',
	'Andrew Morton', 'linuxppc-dev',
	'David S. Miller'
In-Reply-To: <CAPcyv4iJYZBVhV1NW7EB6-EwETiUAy6r1iiE+F+HvFXfGZt9Aw@mail.gmail.com>


> -----Original Message-----
> From: Dan Williams <dan.j.williams@intel.com>
> Sent: Wednesday, 1 April 2020 7:48 PM
> To: Alastair D'Silva <alastair@d-silva.org>
> Cc: Aneesh Kumar K . V <aneesh.kumar@linux.ibm.com>; Oliver O'Halloran
> <oohall@gmail.com>; Benjamin Herrenschmidt
> <benh@kernel.crashing.org>; Paul Mackerras <paulus@samba.org>; Michael
> Ellerman <mpe@ellerman.id.au>; Frederic Barrat <fbarrat@linux.ibm.com>;
> Andrew Donnellan <ajd@linux.ibm.com>; Arnd Bergmann
> <arnd@arndb.de>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>;
> Vishal Verma <vishal.l.verma@intel.com>; Dave Jiang
> <dave.jiang@intel.com>; Ira Weiny <ira.weiny@intel.com>; Andrew Morton
> <akpm@linux-foundation.org>; Mauro Carvalho Chehab
> <mchehab+samsung@kernel.org>; David S. Miller <davem@davemloft.net>;
> Rob Herring <robh@kernel.org>; Anton Blanchard <anton@ozlabs.org>;
> Krzysztof Kozlowski <krzk@kernel.org>; Mahesh Salgaonkar
> <mahesh@linux.vnet.ibm.com>; Madhavan Srinivasan
> <maddy@linux.vnet.ibm.com>; Cédric Le Goater <clg@kaod.org>; Anju T
> Sudhakar <anju@linux.vnet.ibm.com>; Hari Bathini
> <hbathini@linux.ibm.com>; Thomas Gleixner <tglx@linutronix.de>; Greg
> Kurz <groug@kaod.org>; Nicholas Piggin <npiggin@gmail.com>; Masahiro
> Yamada <yamada.masahiro@socionext.com>; Alexey Kardashevskiy
> <aik@ozlabs.ru>; Linux Kernel Mailing List <linux-kernel@vger.kernel.org>;
> linuxppc-dev <linuxppc-dev@lists.ozlabs.org>; linux-nvdimm <linux-
> nvdimm@lists.01.org>; Linux MM <linux-mm@kvack.org>
> Subject: Re: [PATCH v4 00/25] Add support for OpenCAPI Persistent Memory
> devices
> 
> On Sun, Mar 29, 2020 at 10:23 PM Alastair D'Silva <alastair@d-silva.org>
> wrote:
> >
> > This series adds support for OpenCAPI Persistent Memory devices on
> > bare metal (arch/powernv), exposing them as nvdimms so that we can
> > make use of the existing infrastructure. There already exists a driver
> > for the same devices abstracted through PowerVM (arch/pseries):
> > arch/powerpc/platforms/pseries/papr_scm.c
> >
> > These devices are connected via OpenCAPI, and present as LPC (lowest
> coherence point) memory to the system, practically, that means that
> memory on these cards could be treated as conventional, cache-coherent
> memory.
> >
> > Since the devices are connected via OpenCAPI, they are not enumerated
> via ACPI. Instead, OpenCAPI links present as pseudo-PCI bridges, with
> devices below them.
> >
> > This series introduces a driver that exposes the memory on these cards as
> nvdimms, with each card getting it's own bus. This is somewhat complicated
> by the fact that the cards do not have out of band persistent storage for
> metadata, so 1 SECTION_SIZE's (see SPARSEMEM) worth of storage is carved
> out of the top of the card storage to implement the ndctl_config_* calls.
> 
> Is it really tied to section-size? Can't that change based on the configured
> page-size? It's not clear to me why that would be the choice, but I'll dig into
> the implementation.
> 

I had tried using PAGE_SIZE, but ran into problems carving off just 1 page and handing it to the kernel, while leaving the rest as pmem. That was a while ago though, so maybe I should retry it.

> > The driver is not responsible for configuring the NPU (NVLink Processing
> Unit) BARs to map the LPC memory from the card into the system's physical
> address space, instead, it requests this to be done via OPAL calls (typically
> implemented by Skiboot).
> 
> Are OPAL calls similar to ACPI DSMs? I.e. methods for the OS to invoke
> platform firmware services? What's Skiboot?
> 

Yes, OPAL is the interface to firmware for POWER. Skiboot is the open-source (and only) implementation of OPAL.

> >
> > The series is structured as follows:
> >  - Required infrastructure changes & cleanup
> >  - A minimal driver implementation
> >  - Implementing additional features within the driver
> 
> Thanks for the intro and the changelog!
> 
> >
> > Changelog:
> > V4:
> >   - Rebase on next-20200320
> 
> Do you have dependencies on other material that's in -next? Otherwise -
> next is only a viable development baseline if you are going to merge through
> Andrew's tree.
> 
> >   - Bump copyright to 2020
> >   - Ensure all uapi headers use C89 compatible comments (missed
> ocxlpmem.h)
> >   - Move the driver back to drivers/nvdimm/ocxl, after confirmation
> >     that this location is desirable
> >   - Rename ocxl.c to ocxlpmem.c (+ support files)
> >   - Rename all ocxl_pmem to ocxlpmem
> >   - Address checkpatch --strict issues
> >   - "powerpc/powernv: Add OPAL calls for LPC memory alloc/release"
> >         - Pass base address as __be64
> >   - "ocxl: Tally up the LPC memory on a link & allow it to be mapped"
> >         - Address checkpatch spacing warnings
> >         - Reword blurb
> >         - Reword size description for ocxl_link_add_lpc_mem()
> >         - Add an early exit in ocxl_link_lpc_release() to avoid triggering
> >           bogus warnings if called after ocxl_link_lpc_map() fails
> >   - "powerpc/powernv: Add OPAL calls for LPC memory alloc/release"
> >         - Reword blurb
> >   - "powerpc/powernv: Map & release OpenCAPI LPC memory"
> >         - Reword blurb
> >   - Move minor_idr init from file_init() to ocxlpmem_init() (fixes runtime
> error
> >     in "nvdimm: Add driver for OpenCAPI Persistent Memory")
> >   - Wrap long lines
> >   - "nvdimm: Add driver for OpenCAPI Storage Class Memory"
> >         - Remove '+ 1' workround from serial number->cookie assignment
> >         - Drop out of memory message for ocxlpmem in probe()
> >         - Fix leaks of ocxlpmem & ocxlpmem->ocxl_fn in probe()
> >         - remove struct ocxlpmem_function0, it didn't value add
> >         - factor out err_unregistered label in probe
> >         - Address more checkpatch warnings
> >         - get/put the pci dev on probe/free
> >         - Drop ocxlpmem_ prefix from static functions
> >         - Propogate errors up from called functions in probe()
> >         - Set MODULE_LICENSE to GPLv2
> >         - Add myself as module author
> >         - Call nvdimm_bus_unregister() in remove() to release references
> >         - Don't call devm_memunmap on metadata_address, the release
> handler on
> >          the device already deals with this
> >   - "nvdimm/ocxl: Read the capability registers & wait for device ready"
> >         - Fix mask for read_latency
> >         - Fold in is_usable logic into timeout to remove error message race
> >         - propogate bad rc from read_device_metadata
> >   - "nvdimm/ocxl: Add register addresses & status values to the header"
> >         - Add comments for register abbreviations where names have been
> >           expanded
> >         - Add missing status for blocked on background task
> >         - Alias defines for firmware update status to show that the duplication
> >           of values is intentional
> >   - "nvdimm/ocxl: Register a character device for userspace to interact with"
> >         - Add lock around minors IDR, delete the cdev before
> device_unregister
> >         - Propogate errors up from called functions in probe()
> >   - "nvdimm/ocxl: Add support for Admin commands"
> >         - Fix typo in setup_command_data error message, and drop 'ocxl' from
> it
> >         - Drop vestigial CHI read from admin_command_request
> >         - Change command ID mismatch message to dev_err, and return an
> error
> >         - Use jiffies to implement admin_command_complete_timeout()
> >         - Flesh out blurb
> >         - Create a wrapper to issue the command & wait for timeout
> >   - "nvdimm/ocxl: Add support for near storage commands"
> >         - dropped (will submit with the patches for nvdimm overwrite)
> >   - "nvdimm/ocxl: Implement the Read Error Log command"
> >         - Remove stray blank line
> >         - change misplaced goto to an early exit in read_error_log
> >         - Inline error_log_offset_0x08
> >         - Read WWID data as LE rather than host endian
> >         - Move the include of nvdimm/ocxlpmem.h to ocxl.c
> >         - Add padding after fwrevision in struct ioctl_ocxl_pmem_error_log
> >         - Register IOCTL magic
> >         - Coerce pointers to __u64 in IOCTLs
> >   - "nvdimm/ocxl: Add controller dump IOCTLs"
> >         - Coerce pointers to __u64 in IOCTLs
> >         - Document expected IOCTL usage in blurb
> >         - Add missing rc check
> >         - Only populate up to the number of bytes returned by the card,
> >           and return this length to the caller
> >         - Add missing header check
> >   - "nvdimm/ocxl: Add an IOCTL to report controller statistics"
> >         - Update to match the latest version of the spec
> >         - Verify that parametr block IDs & lengths match what we expect
> >         - Use defines for offsets
> >   - "nvdimm/ocxl: Forward events to userspace"
> >         - Don't enable NSCRA doorbell
> >         - return -EBUSY if the event context is already used
> >         - return -ENODEV if IRQs cannot be mapped
> >         - Tag IRQ pointers with __iomem
> >         - Drop ocxlpmem_ prefix from static functions
> >         - Propogate error from eventfd_ctx_fdget
> >         - Fix error check in copy_to_user
> >         - Drop GLOBAL_MMIO_CHI_NSCRA (this should be in the overwrite
> patch)
> >         - Drop unused irq_pgmap
> >         - Don't redef BIT_ULL
> >   - "nvdimm/ocxl: Add debug IOCTLs"
> >         - Eliminate clearing loop (now done in admin_command_execute()
> >         - Drop dummy IOCTLs if CONFIG_OCXL_PMEM_DEBUG is not set
> >         - Group debug IOCTLs together & comment that they may not be
> available
> >   - "nvdimm/ocxl: Expose SMART data via ndctl"
> >         - Drop 'rc = 0; goto out;'
> >         - Propogate errors from ndctl_smart()
> >   - "nvdimm/ocxl: Expose the serial number in sysfs" & "nvdimm/ocxl:
> Expose the firmware version in sysfs"
> >         - Squash these 2 patches together
> >         - Expose data as a DIMM attribute rather than an ocxlpmem
> >           attribute
> >   - "nvdimm/ocxl: Add an IOCTL to request controller health & perf data"
> >         - Reword blurb
> >   - "nvdimm/ocxl: Implement the heartbeat command"
> >         - Propogate rc in probe()
> >
> > V3:
> >   - Rebase against next/next-20200220
> >   - Move driver to arch/powerpc/platforms/powernv, we now expect this
> >     driver to go upstream via the powerpc tree
> >   - "nvdimm/ocxl: Implement the Read Error Log command"
> >         - Fix bad header path
> >   - "nvdimm/ocxl: Read the capability registers & wait for device ready"
> >         - Fix overlapping masks between readiness_timeout &
> memory_available_timeout
> >   - "nvdimm: Add driver for OpenCAPI Storage Class Memory"
> >         - Address minor review comments from Jonathan Cameron
> >         - Remove attributes
> >         - Default to module if building LIBNVDIMM
> >         - Propogate errors up from called functions in probe()
> >   - "nvdimm/ocxl: Expose SMART data via ndctl"
> >         - Pack attributes in struct
> >         - Support different size SMART buffers for compatibility with newer
> >           ndctls that may want more SMART attribs than we provide
> >         - Rework to to use ND_CMD_CALL instead of ND_CMD_SMART
> >   - drop "ocxl: Free detached contexts in ocxl_context_detach_all()"
> >   - "powerpc: Map & release OpenCAPI LPC memory"
> >         - Remove 'extern'
> >         - Only available with CONFIG_MEMORY_HOTPLUG_SPARSE
> >   - "ocxl: Tally up the LPC memory on a link & allow it to be mapped"
> >         - Address minor review comments from Jonathan Cameron
> >   - "ocxl: Add functions to map/unmap LPC memory"
> >         - Split detected memory message into a separate patch
> >         - Address minor review comments from Jonathan Cameron
> >         - Add a comment explaining why unmap_lpc_mem is in
> deconfigure_afu
> >   - "nvdimm/ocxl: Add support for Admin commands"
> >         - use sizeof(u64) rather than 0x08 when iterating u64s
> >   - "nvdimm/ocxl: Implement the heartbeat command"
> >         - Fix typo in blurb
> >   - Address kernel doc issues
> >   - Ensure all uapi headers use C89 compatible comments
> >   - Drop patches for firmware update & overwrite, these will be
> >     submitted later once patches are available for ndctl
> >   - Rename SCM to OpenCAPI Persistent Memory
> >
> > V2:
> >   - "powerpc: Map & release OpenCAPI LPC memory"
> >       - Fix #if -> #ifdef
> >       - use pci_dev_id to get the bdfn
> >       - use __be64 to hold be data
> >       - indent check_hotplug_memory_addressable correctly
> >       - Remove export of check_hotplug_memory_addressable
> >   - "ocxl: Conditionally bind SCM devices to the generic OCXL driver"
> >       - Improve patch description and remove redundant default
> >   - "nvdimm: Add driver for OpenCAPI Storage Class Memory"
> >       - Mark a few funcs as static as identified by the 0day bot
> >       - Add OCXL dependancies to OCXL_SCM
> >       - Use memcpy_mcsafe in scm_ndctl_config_read
> >       - Rename scm_foo_offset_0x00 to scm_foo_header_parse & add docs
> >       - Name DIMM attribs "ocxl" rather than "scm"
> >       - Split out into base + many feature patches
> >   - "powerpc: Enable OpenCAPI Storage Class Memory driver on bare
> metal"
> >       - Build DEV_DAX & friends as modules
> >   - "ocxl: Conditionally bind SCM devices to the generic OCXL driver"
> >       - Patch dropped (easy enough to maintain this out of tree for
> development)
> >   - "ocxl: Tally up the LPC memory on a link & allow it to be mapped"
> >       - Add a warning if an unmatched lpc_release is called
> >   - "ocxl: Add functions to map/unmap LPC memory"
> >       - Use EXPORT_SYMBOL_GPL
> >
> >
> > Alastair D'Silva (25):
> >   powerpc/powernv: Add OPAL calls for LPC memory alloc/release
> >   mm/memory_hotplug: Allow check_hotplug_memory_addressable to be
> called
> >     from drivers
> >   powerpc/powernv: Map & release OpenCAPI LPC memory
> >   ocxl: Remove unnecessary externs
> >   ocxl: Address kernel doc errors & warnings
> >   ocxl: Tally up the LPC memory on a link & allow it to be mapped
> >   ocxl: Add functions to map/unmap LPC memory
> >   ocxl: Emit a log message showing how much LPC memory was detected
> >   ocxl: Save the device serial number in ocxl_fn
> >   nvdimm: Add driver for OpenCAPI Persistent Memory
> >   powerpc: Enable the OpenCAPI Persistent Memory driver for
> >     powernv_defconfig
> >   nvdimm/ocxl: Add register addresses & status values to the header
> >   nvdimm/ocxl: Read the capability registers & wait for device ready
> >   nvdimm/ocxl: Add support for Admin commands
> >   nvdimm/ocxl: Register a character device for userspace to interact
> >     with
> >   nvdimm/ocxl: Implement the Read Error Log command
> >   nvdimm/ocxl: Add controller dump IOCTLs
> >   nvdimm/ocxl: Add an IOCTL to report controller statistics
> >   nvdimm/ocxl: Forward events to userspace
> >   nvdimm/ocxl: Add an IOCTL to request controller health & perf data
> >   nvdimm/ocxl: Implement the heartbeat command
> >   nvdimm/ocxl: Add debug IOCTLs
> >   nvdimm/ocxl: Expose SMART data via ndctl
> >   nvdimm/ocxl: Expose the serial number & firmware version in sysfs
> >   MAINTAINERS: Add myself & nvdimm/ocxl to ocxl
> >
> >  .../userspace-api/ioctl/ioctl-number.rst      |    1 +
> >  MAINTAINERS                                   |    3 +
> >  arch/powerpc/configs/powernv_defconfig        |    5 +
> >  arch/powerpc/include/asm/opal-api.h           |    2 +
> >  arch/powerpc/include/asm/opal.h               |    2 +
> >  arch/powerpc/include/asm/pnv-ocxl.h           |   42 +-
> >  arch/powerpc/platforms/powernv/ocxl.c         |   43 +
> >  arch/powerpc/platforms/powernv/opal-call.c    |    2 +
> >  drivers/misc/ocxl/config.c                    |   74 +-
> >  drivers/misc/ocxl/core.c                      |   61 +
> >  drivers/misc/ocxl/link.c                      |   60 +
> >  drivers/misc/ocxl/ocxl_internal.h             |   45 +-
> >  drivers/nvdimm/Kconfig                        |    2 +
> >  drivers/nvdimm/Makefile                       |    1 +
> >  drivers/nvdimm/ocxl/Kconfig                   |   21 +
> >  drivers/nvdimm/ocxl/Makefile                  |    7 +
> >  drivers/nvdimm/ocxl/main.c                    | 1975 +++++++++++++++++
> >  drivers/nvdimm/ocxl/ocxlpmem.h                |  197 ++
> >  drivers/nvdimm/ocxl/ocxlpmem_internal.c       |  280 +++
> >  include/linux/memory_hotplug.h                |    5 +
> >  include/misc/ocxl.h                           |  122 +-
> >  include/uapi/linux/ndctl.h                    |    1 +
> >  include/uapi/nvdimm/ocxlpmem.h                |  127 ++
> >  mm/memory_hotplug.c                           |    4 +-
> >  24 files changed, 2983 insertions(+), 99 deletions(-)  create mode
> > 100644 drivers/nvdimm/ocxl/Kconfig  create mode 100644
> > drivers/nvdimm/ocxl/Makefile  create mode 100644
> > drivers/nvdimm/ocxl/main.c  create mode 100644
> > drivers/nvdimm/ocxl/ocxlpmem.h  create mode 100644
> > drivers/nvdimm/ocxl/ocxlpmem_internal.c
> >  create mode 100644 include/uapi/nvdimm/ocxlpmem.h
> >
> > --
> > 2.24.1
> >


-- 
Alastair D'Silva           mob: 0423 762 819
skype: alastair_dsilva     msn: alastair@d-silva.org
blog: http://alastair.d-silva.org    Twitter: @EvilDeece


^ permalink raw reply

* Re: [PATCH v2 1/1] vfio-pci/nvlink2: Allow fallback to ibm,mmio-atsd[0]
From: Alex Williamson @ 2020-04-01 22:39 UTC (permalink / raw)
  To: Sam Bobroff; +Cc: aik, linuxppc-dev, kvm
In-Reply-To: <6183bf8ec2dd0433f213e081911ab8fd5cac2dcb.1585627961.git.sbobroff@linux.ibm.com>

On Tue, 31 Mar 2020 15:12:46 +1100
Sam Bobroff <sbobroff@linux.ibm.com> wrote:

> Older versions of skiboot only provide a single value in the device
> tree property "ibm,mmio-atsd", even when multiple Address Translation
> Shoot Down (ATSD) registers are present. This prevents NVLink2 devices
> (other than the first) from being used with vfio-pci because vfio-pci
> expects to be able to assign a dedicated ATSD register to each NVLink2
> device.
> 
> However, ATSD registers can be shared among devices. This change
> allows vfio-pci to fall back to sharing the register at index 0 if
> necessary.
> 
> Fixes: 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] subdriver")
> Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
> ---
> Patch set v2:
> Patch 1/1: vfio-pci/nvlink2: Allow fallback to ibm,mmio-atsd[0]
> - Removed unnecessary warning.
> - Added Fixes tag.
> 
> Patch set v1:
> Patch 1/1: vfio-pci/nvlink2: Allow fallback to ibm,mmio-atsd[0]
> 
>  drivers/vfio/pci/vfio_pci_nvlink2.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)

Applied to vfio next branch for v5.7 with Alexey's review.  Thanks,

Alex

> diff --git a/drivers/vfio/pci/vfio_pci_nvlink2.c b/drivers/vfio/pci/vfio_pci_nvlink2.c
> index f2983f0f84be..ae2af590e501 100644
> --- a/drivers/vfio/pci/vfio_pci_nvlink2.c
> +++ b/drivers/vfio/pci/vfio_pci_nvlink2.c
> @@ -420,8 +420,14 @@ int vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev)
>  
>  	if (of_property_read_u64_index(hose->dn, "ibm,mmio-atsd", nvlink_index,
>  			&mmio_atsd)) {
> -		dev_warn(&vdev->pdev->dev, "No available ATSD found\n");
> -		mmio_atsd = 0;
> +		if (of_property_read_u64_index(hose->dn, "ibm,mmio-atsd", 0,
> +				&mmio_atsd)) {
> +			dev_warn(&vdev->pdev->dev, "No available ATSD found\n");
> +			mmio_atsd = 0;
> +		} else {
> +			dev_warn(&vdev->pdev->dev,
> +				 "Using fallback ibm,mmio-atsd[0] for ATSD.\n");
> +		}
>  	}
>  
>  	if (of_property_read_u64(npu_node, "ibm,device-tgt-addr", &tgt)) {


^ permalink raw reply

* [PATCH v5 6/9] crypto/nx: Make enable code generic to add new GZIP compression type
From: Haren Myneni @ 2020-04-01 22:16 UTC (permalink / raw)
  To: mpe; +Cc: mikey, herbert, npiggin, linux-crypto, sukadev, linuxppc-dev, dja
In-Reply-To: <1585778775.2275.2.camel@hbabu-laptop>

(Sorry for reposting. version number is missed in subject)

Make setup and enable code generic to support new GZIP compression type.
Changed nx842 reference to nx and moved some code to new functions.
Functionality is not changed except sparse warning fix - setting NULL
instead of 0 for per_cpu send window in nx_delete_coprocs().

Signed-off-by: Haren Myneni <haren@linux.ibm.com>
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
---
 drivers/crypto/nx/nx-common-powernv.c | 161 +++++++++++++++++++++-------------
 1 file changed, 101 insertions(+), 60 deletions(-)

diff --git a/drivers/crypto/nx/nx-common-powernv.c b/drivers/crypto/nx/nx-common-powernv.c
index f42881f..82dfa60 100644
--- a/drivers/crypto/nx/nx-common-powernv.c
+++ b/drivers/crypto/nx/nx-common-powernv.c
@@ -40,9 +40,9 @@ struct nx842_workmem {
 	char padding[WORKMEM_ALIGN]; /* unused, to allow alignment */
 } __packed __aligned(WORKMEM_ALIGN);
 
-struct nx842_coproc {
+struct nx_coproc {
 	unsigned int chip_id;
-	unsigned int ct;
+	unsigned int ct;	/* Can be 842 or GZIP high/normal*/
 	unsigned int ci;	/* Coprocessor instance, used with icswx */
 	struct {
 		struct vas_window *rxwin;
@@ -58,9 +58,15 @@ struct nx842_coproc {
 static DEFINE_PER_CPU(struct vas_window *, cpu_txwin);
 
 /* no cpu hotplug on powernv, so this list never changes after init */
-static LIST_HEAD(nx842_coprocs);
+static LIST_HEAD(nx_coprocs);
 static unsigned int nx842_ct;	/* used in icswx function */
 
+/*
+ * Using same values as in skiboot or coprocessor type representing
+ * in NX workbook.
+ */
+#define NX_CT_842	(3)
+
 static int (*nx842_powernv_exec)(const unsigned char *in,
 				unsigned int inlen, unsigned char *out,
 				unsigned int *outlenp, void *workmem, int fc);
@@ -666,15 +672,15 @@ static int nx842_powernv_decompress(const unsigned char *in, unsigned int inlen,
 				      wmem, CCW_FC_842_DECOMP_CRC);
 }
 
-static inline void nx842_add_coprocs_list(struct nx842_coproc *coproc,
+static inline void nx_add_coprocs_list(struct nx_coproc *coproc,
 					int chipid)
 {
 	coproc->chip_id = chipid;
 	INIT_LIST_HEAD(&coproc->list);
-	list_add(&coproc->list, &nx842_coprocs);
+	list_add(&coproc->list, &nx_coprocs);
 }
 
-static struct vas_window *nx842_alloc_txwin(struct nx842_coproc *coproc)
+static struct vas_window *nx_alloc_txwin(struct nx_coproc *coproc)
 {
 	struct vas_window *txwin = NULL;
 	struct vas_tx_win_attr txattr;
@@ -704,9 +710,9 @@ static struct vas_window *nx842_alloc_txwin(struct nx842_coproc *coproc)
  * cpu_txwin is used in copy/paste operation for each compression /
  * decompression request.
  */
-static int nx842_open_percpu_txwins(void)
+static int nx_open_percpu_txwins(void)
 {
-	struct nx842_coproc *coproc, *n;
+	struct nx_coproc *coproc, *n;
 	unsigned int i, chip_id;
 
 	for_each_possible_cpu(i) {
@@ -714,17 +720,18 @@ static int nx842_open_percpu_txwins(void)
 
 		chip_id = cpu_to_chip_id(i);
 
-		list_for_each_entry_safe(coproc, n, &nx842_coprocs, list) {
+		list_for_each_entry_safe(coproc, n, &nx_coprocs, list) {
 			/*
 			 * Kernel requests use only high priority FIFOs. So
 			 * open send windows for these FIFOs.
+			 * GZIP is not supported in kernel right now.
 			 */
 
 			if (coproc->ct != VAS_COP_TYPE_842_HIPRI)
 				continue;
 
 			if (coproc->chip_id == chip_id) {
-				txwin = nx842_alloc_txwin(coproc);
+				txwin = nx_alloc_txwin(coproc);
 				if (IS_ERR(txwin))
 					return PTR_ERR(txwin);
 
@@ -743,13 +750,28 @@ static int nx842_open_percpu_txwins(void)
 	return 0;
 }
 
+static int __init nx_set_ct(struct nx_coproc *coproc, const char *priority,
+				int high, int normal)
+{
+	if (!strcmp(priority, "High"))
+		coproc->ct = high;
+	else if (!strcmp(priority, "Normal"))
+		coproc->ct = normal;
+	else {
+		pr_err("Invalid RxFIFO priority value\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int __init vas_cfg_coproc_info(struct device_node *dn, int chip_id,
-					int vasid, int *ct)
+					int vasid, int type, int *ct)
 {
 	struct vas_window *rxwin = NULL;
 	struct vas_rx_win_attr rxattr;
-	struct nx842_coproc *coproc;
 	u32 lpid, pid, tid, fifo_size;
+	struct nx_coproc *coproc;
 	u64 rx_fifo;
 	const char *priority;
 	int ret;
@@ -794,15 +816,12 @@ static int __init vas_cfg_coproc_info(struct device_node *dn, int chip_id,
 	if (!coproc)
 		return -ENOMEM;
 
-	if (!strcmp(priority, "High"))
-		coproc->ct = VAS_COP_TYPE_842_HIPRI;
-	else if (!strcmp(priority, "Normal"))
-		coproc->ct = VAS_COP_TYPE_842;
-	else {
-		pr_err("Invalid RxFIFO priority value\n");
-		ret =  -EINVAL;
+	if (type == NX_CT_842)
+		ret = nx_set_ct(coproc, priority, VAS_COP_TYPE_842_HIPRI,
+			VAS_COP_TYPE_842);
+
+	if (ret)
 		goto err_out;
-	}
 
 	vas_init_rx_win_attr(&rxattr, coproc->ct);
 	rxattr.rx_fifo = (void *)rx_fifo;
@@ -830,7 +849,7 @@ static int __init vas_cfg_coproc_info(struct device_node *dn, int chip_id,
 
 	coproc->vas.rxwin = rxwin;
 	coproc->vas.id = vasid;
-	nx842_add_coprocs_list(coproc, chip_id);
+	nx_add_coprocs_list(coproc, chip_id);
 
 	/*
 	 * (lpid, pid, tid) combination has to be unique for each
@@ -848,13 +867,43 @@ static int __init vas_cfg_coproc_info(struct device_node *dn, int chip_id,
 	return ret;
 }
 
+static int __init nx_coproc_init(int chip_id, int ct_842)
+{
+	int ret = 0;
 
-static int __init nx842_powernv_probe_vas(struct device_node *pn)
+	if (opal_check_token(OPAL_NX_COPROC_INIT)) {
+		ret = opal_nx_coproc_init(chip_id, ct_842);
+		if (ret) {
+			ret = opal_error_code(ret);
+			pr_err("Failed to initialize NX for chip(%d): %d\n",
+				chip_id, ret);
+		}
+	} else
+		pr_warn("Firmware doesn't support NX initialization\n");
+
+	return ret;
+}
+
+static int __init find_nx_device_tree(struct device_node *dn, int chip_id,
+					int vasid, int type, char *devname,
+					int *ct)
+{
+	int ret = 0;
+
+	if (of_device_is_compatible(dn, devname)) {
+		ret  = vas_cfg_coproc_info(dn, chip_id, vasid, type, ct);
+		if (ret)
+			of_node_put(dn);
+	}
+
+	return ret;
+}
+
+static int __init nx_powernv_probe_vas(struct device_node *pn)
 {
-	struct device_node *dn;
 	int chip_id, vasid, ret = 0;
-	int nx_fifo_found = 0;
-	int uninitialized_var(ct);
+	struct device_node *dn;
+	int ct_842 = 0;
 
 	chip_id = of_get_ibm_chip_id(pn);
 	if (chip_id < 0) {
@@ -869,17 +918,13 @@ static int __init nx842_powernv_probe_vas(struct device_node *pn)
 	}
 
 	for_each_child_of_node(pn, dn) {
-		if (of_device_is_compatible(dn, "ibm,p9-nx-842")) {
-			ret = vas_cfg_coproc_info(dn, chip_id, vasid, &ct);
-			if (ret) {
-				of_node_put(dn);
-				return ret;
-			}
-			nx_fifo_found++;
-		}
+		ret = find_nx_device_tree(dn, chip_id, vasid, NX_CT_842,
+					"ibm,p9-nx-842", &ct_842);
+		if (ret)
+			return ret;
 	}
 
-	if (!nx_fifo_found) {
+	if (!ct_842) {
 		pr_err("NX842 FIFO nodes are missing\n");
 		return -EINVAL;
 	}
@@ -887,22 +932,14 @@ static int __init nx842_powernv_probe_vas(struct device_node *pn)
 	/*
 	 * Initialize NX instance for both high and normal priority FIFOs.
 	 */
-	if (opal_check_token(OPAL_NX_COPROC_INIT)) {
-		ret = opal_nx_coproc_init(chip_id, ct);
-		if (ret) {
-			pr_err("Failed to initialize NX for chip(%d): %d\n",
-				chip_id, ret);
-			ret = opal_error_code(ret);
-		}
-	} else
-		pr_warn("Firmware doesn't support NX initialization\n");
+	ret = nx_coproc_init(chip_id, ct_842);
 
 	return ret;
 }
 
 static int __init nx842_powernv_probe(struct device_node *dn)
 {
-	struct nx842_coproc *coproc;
+	struct nx_coproc *coproc;
 	unsigned int ct, ci;
 	int chip_id;
 
@@ -928,7 +965,7 @@ static int __init nx842_powernv_probe(struct device_node *dn)
 
 	coproc->ct = ct;
 	coproc->ci = ci;
-	nx842_add_coprocs_list(coproc, chip_id);
+	nx_add_coprocs_list(coproc, chip_id);
 
 	pr_info("coprocessor found on chip %d, CT %d CI %d\n", chip_id, ct, ci);
 
@@ -941,9 +978,9 @@ static int __init nx842_powernv_probe(struct device_node *dn)
 	return 0;
 }
 
-static void nx842_delete_coprocs(void)
+static void nx_delete_coprocs(void)
 {
-	struct nx842_coproc *coproc, *n;
+	struct nx_coproc *coproc, *n;
 	struct vas_window *txwin;
 	int i;
 
@@ -955,10 +992,10 @@ static void nx842_delete_coprocs(void)
 		if (txwin)
 			vas_win_close(txwin);
 
-		per_cpu(cpu_txwin, i) = 0;
+		per_cpu(cpu_txwin, i) = NULL;
 	}
 
-	list_for_each_entry_safe(coproc, n, &nx842_coprocs, list) {
+	list_for_each_entry_safe(coproc, n, &nx_coprocs, list) {
 		if (coproc->vas.rxwin)
 			vas_win_close(coproc->vas.rxwin);
 
@@ -1002,7 +1039,7 @@ static int nx842_powernv_crypto_init(struct crypto_tfm *tfm)
 	.coa_decompress		= nx842_crypto_decompress } }
 };
 
-static __init int nx842_powernv_init(void)
+static __init int nx_compress_powernv_init(void)
 {
 	struct device_node *dn;
 	int ret;
@@ -1017,15 +1054,15 @@ static __init int nx842_powernv_init(void)
 	BUILD_BUG_ON(DDE_BUFFER_SIZE_MULT % DDE_BUFFER_LAST_MULT);
 
 	for_each_compatible_node(dn, NULL, "ibm,power9-nx") {
-		ret = nx842_powernv_probe_vas(dn);
+		ret = nx_powernv_probe_vas(dn);
 		if (ret) {
-			nx842_delete_coprocs();
+			nx_delete_coprocs();
 			of_node_put(dn);
 			return ret;
 		}
 	}
 
-	if (list_empty(&nx842_coprocs)) {
+	if (list_empty(&nx_coprocs)) {
 		for_each_compatible_node(dn, NULL, "ibm,power-nx")
 			nx842_powernv_probe(dn);
 
@@ -1034,9 +1071,13 @@ static __init int nx842_powernv_init(void)
 
 		nx842_powernv_exec = nx842_exec_icswx;
 	} else {
-		ret = nx842_open_percpu_txwins();
+		/*
+		 * GZIP is not supported in kernel right now.
+		 * So open tx windows only for 842.
+		 */
+		ret = nx_open_percpu_txwins();
 		if (ret) {
-			nx842_delete_coprocs();
+			nx_delete_coprocs();
 			return ret;
 		}
 
@@ -1045,18 +1086,18 @@ static __init int nx842_powernv_init(void)
 
 	ret = crypto_register_alg(&nx842_powernv_alg);
 	if (ret) {
-		nx842_delete_coprocs();
+		nx_delete_coprocs();
 		return ret;
 	}
 
 	return 0;
 }
-module_init(nx842_powernv_init);
+module_init(nx_compress_powernv_init);
 
-static void __exit nx842_powernv_exit(void)
+static void __exit nx_compress_powernv_exit(void)
 {
 	crypto_unregister_alg(&nx842_powernv_alg);
 
-	nx842_delete_coprocs();
+	nx_delete_coprocs();
 }
-module_exit(nx842_powernv_exit);
+module_exit(nx_compress_powernv_exit);
-- 
1.8.3.1




^ permalink raw reply related

* [PATCH v9 06/13] powerpc/vas: Take reference to PID and mm for user space windows
From: Haren Myneni @ 2020-04-01 22:13 UTC (permalink / raw)
  To: mpe
  Cc: mikey, ajd, frederic.barrat, linux-kernel, npiggin, hch, oohall,
	clg, herbert, sukadev, linuxppc-dev, srikar
In-Reply-To: <1585776497.10664.448.camel@hbabu-laptop>

(sorry reposting. version string missed)

When process opens a window, its pid and tgid will be saved in the
vas_window struct. This window will be closed when the process exits.
The kernel handles NX faults by updating CSB or send SEGV signal to pid
of the process if the user space csb addr is invalid.

In multi-thread applications, a window can be opened by a child thread,
but it will not be closed when this thread exits. It is expected that
the parent will clean up all resources including NX windows opened by
child threads. A child thread can send NX requests using this window
and could be killed before completion is reported. If the pid assigned
to this thread is reused while requests are pending, a failure SEGV
would be directed to the wrong place.

To prevent reusing the pid, take references to pid and mm when the window
is opened and release them when when the window is closed. Then if child
thread is not running, SEGV signal will be sent to thread group leader
(tgid).

Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/platforms/powernv/vas-debug.c  |  2 +-
 arch/powerpc/platforms/powernv/vas-window.c | 53 ++++++++++++++++++++++++++---
 arch/powerpc/platforms/powernv/vas.h        |  9 ++++-
 3 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/vas-debug.c b/arch/powerpc/platforms/powernv/vas-debug.c
index 09e63df..ef9a717 100644
--- a/arch/powerpc/platforms/powernv/vas-debug.c
+++ b/arch/powerpc/platforms/powernv/vas-debug.c
@@ -38,7 +38,7 @@ static int info_show(struct seq_file *s, void *private)
 
 	seq_printf(s, "Type: %s, %s\n", cop_to_str(window->cop),
 					window->tx_win ? "Send" : "Receive");
-	seq_printf(s, "Pid : %d\n", window->pid);
+	seq_printf(s, "Pid : %d\n", vas_window_pid(window));
 
 unlock:
 	mutex_unlock(&vas_mutex);
diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
index dc46bf6..7054cd4 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -12,6 +12,8 @@
 #include <linux/log2.h>
 #include <linux/rcupdate.h>
 #include <linux/cred.h>
+#include <linux/sched/mm.h>
+#include <linux/mmu_context.h>
 #include <asm/switch_to.h>
 #include <asm/ppc-opcode.h>
 #include "vas.h"
@@ -876,8 +878,6 @@ struct vas_window *vas_rx_win_open(int vasid, enum vas_cop_type cop,
 	rxwin->user_win = rxattr->user_win;
 	rxwin->cop = cop;
 	rxwin->wcreds_max = rxattr->wcreds_max ?: VAS_WCREDS_DEFAULT;
-	if (rxattr->user_win)
-		rxwin->pid = task_pid_vnr(current);
 
 	init_winctx_for_rxwin(rxwin, rxattr, &winctx);
 	init_winctx_regs(rxwin, &winctx);
@@ -1027,7 +1027,6 @@ struct vas_window *vas_tx_win_open(int vasid, enum vas_cop_type cop,
 	txwin->tx_win = 1;
 	txwin->rxwin = rxwin;
 	txwin->nx_win = txwin->rxwin->nx_win;
-	txwin->pid = attr->pid;
 	txwin->user_win = attr->user_win;
 	txwin->wcreds_max = attr->wcreds_max ?: VAS_WCREDS_DEFAULT;
 
@@ -1059,8 +1058,43 @@ struct vas_window *vas_tx_win_open(int vasid, enum vas_cop_type cop,
 			goto free_window;
 	}
 
-	set_vinst_win(vinst, txwin);
+	if (txwin->user_win) {
+		/*
+		 * Window opened by a child thread may not be closed when
+		 * it exits. So take reference to its pid and release it
+		 * when the window is free by parent thread.
+		 * Acquire a reference to the task's pid to make sure
+		 * pid will not be re-used - needed only for multithread
+		 * applications.
+		 */
+		txwin->pid = get_task_pid(current, PIDTYPE_PID);
+		/*
+		 * Acquire a reference to the task's mm.
+		 */
+		txwin->mm = get_task_mm(current);
 
+		if (!txwin->mm) {
+			put_pid(txwin->pid);
+			pr_err("VAS: pid(%d): mm_struct is not found\n",
+					current->pid);
+			rc = -EPERM;
+			goto free_window;
+		}
+
+		mmgrab(txwin->mm);
+		mmput(txwin->mm);
+		mm_context_add_copro(txwin->mm);
+		/*
+		 * Process closes window during exit. In the case of
+		 * multithread application, the child thread can open
+		 * window and can exit without closing it. Expects parent
+		 * thread to use and close the window. So do not need
+		 * to take pid reference for parent thread.
+		 */
+		txwin->tgid = find_get_pid(task_tgid_vnr(current));
+	}
+
+	set_vinst_win(vinst, txwin);
 	return txwin;
 
 free_window:
@@ -1257,8 +1291,17 @@ int vas_win_close(struct vas_window *window)
 	poll_window_castout(window);
 
 	/* if send window, drop reference to matching receive window */
-	if (window->tx_win)
+	if (window->tx_win) {
+		if (window->user_win) {
+			/* Drop references to pid and mm */
+			put_pid(window->pid);
+			if (window->mm) {
+				mm_context_remove_copro(window->mm);
+				mmdrop(window->mm);
+			}
+		}
 		put_rx_win(window->rxwin);
+	}
 
 	vas_window_free(window);
 
diff --git a/arch/powerpc/platforms/powernv/vas.h b/arch/powerpc/platforms/powernv/vas.h
index 88d084d..2a04072 100644
--- a/arch/powerpc/platforms/powernv/vas.h
+++ b/arch/powerpc/platforms/powernv/vas.h
@@ -355,7 +355,9 @@ struct vas_window {
 	bool user_win;		/* True if user space window */
 	void *hvwc_map;		/* HV window context */
 	void *uwc_map;		/* OS/User window context */
-	pid_t pid;		/* Linux process id of owner */
+	struct pid *pid;	/* Linux process id of owner */
+	struct pid *tgid;	/* Thread group ID of owner */
+	struct mm_struct *mm;	/* Linux process mm_struct */
 	int wcreds_max;		/* Window credits */
 
 	char *dbgname;
@@ -430,6 +432,11 @@ struct vas_winctx {
 extern void vas_window_free_dbgdir(struct vas_window *win);
 extern int vas_setup_fault_window(struct vas_instance *vinst);
 
+static inline int vas_window_pid(struct vas_window *window)
+{
+	return pid_vnr(window->pid);
+}
+
 static inline void vas_log_write(struct vas_window *win, char *name,
 			void *regptr, u64 val)
 {
-- 
1.8.3.1




^ permalink raw reply related

* [PATCH v5 9/9] Documentation/powerpc: VAS API
From: Haren Myneni @ 2020-04-01 22:08 UTC (permalink / raw)
  To: mpe; +Cc: mikey, herbert, npiggin, linux-crypto, sukadev, linuxppc-dev, dja
In-Reply-To: <1585777592.10664.462.camel@hbabu-laptop>


Power9 introduced Virtual Accelerator Switchboard (VAS) which allows
user space to communicate with Nest Accelerator (NX) directly. But
kernel has to establish channel to NX for user space. This document
describes user space API that application can use to establish
communication channel.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.ibm.com>
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 Documentation/powerpc/index.rst   |   1 +
 Documentation/powerpc/vas-api.rst | 292 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 293 insertions(+)
 create mode 100644 Documentation/powerpc/vas-api.rst

diff --git a/Documentation/powerpc/index.rst b/Documentation/powerpc/index.rst
index 0d45f0f..afe2d5e 100644
--- a/Documentation/powerpc/index.rst
+++ b/Documentation/powerpc/index.rst
@@ -30,6 +30,7 @@ powerpc
     syscall64-abi
     transactional_memory
     ultravisor
+    vas-api
 
 .. only::  subproject and html
 
diff --git a/Documentation/powerpc/vas-api.rst b/Documentation/powerpc/vas-api.rst
new file mode 100644
index 0000000..1217c2f
--- /dev/null
+++ b/Documentation/powerpc/vas-api.rst
@@ -0,0 +1,292 @@
+.. SPDX-License-Identifier: GPL-2.0
+.. _VAS-API:
+
+===================================================
+Virtual Accelerator Switchboard (VAS) userspace API
+===================================================
+
+Introduction
+============
+
+Power9 processor introduced Virtual Accelerator Switchboard (VAS) which
+allows both userspace and kernel communicate to co-processor
+(hardware accelerator) referred to as the Nest Accelerator (NX). The NX
+unit comprises of one or more hardware engines or co-processor types
+such as 842 compression, GZIP compression and encryption. On power9,
+userspace applications will have access to only GZIP Compression engine
+which supports ZLIB and GZIP compression algorithms in the hardware.
+
+To communicate with NX, kernel has to establish a channel or window and
+then requests can be submitted directly without kernel involvement.
+Requests to the GZIP engine must be formatted as a co-processor Request
+Block (CRB) and these CRBs must be submitted to the NX using COPY/PASTE
+instructions to paste the CRB to hardware address that is associated with
+the engine's request queue.
+
+The GZIP engine provides two priority levels of requests: Normal and
+High. Only Normal requests are supported from userspace right now.
+
+This document explains userspace API that is used to interact with
+kernel to setup channel / window which can be used to send compression
+requests directly to NX accelerator.
+
+
+Overview
+========
+
+Application access to the GZIP engine is provided through
+/dev/crypto/nx-gzip device node implemented by the VAS/NX device driver.
+An application must open the /dev/crypto/nx-gzip device to obtain a file
+descriptor (fd). Then should issue VAS_TX_WIN_OPEN ioctl with this fd to
+establish connection to the engine. It means send window is opened on GZIP
+engine for this process. Once a connection is established, the application
+should use the mmap() system call to map the hardware address of engine's
+request queue into the application's virtual address space.
+
+The application can then submit one or more requests to the the engine by
+using copy/paste instructions and pasting the CRBs to the virtual address
+(aka paste_address) returned by mmap(). User space can close the
+established connection or send window by closing the file descriptior
+(close(fd)) or upon the process exit.
+
+Note that applications can send several requests with the same window or
+can establish multiple windows, but one window for each file descriptor.
+
+Following sections provide additional details and references about the
+individual steps.
+
+NX-GZIP Device Node
+===================
+
+There is one /dev/crypto/nx-gzip node in the system and it provides
+access to all GZIP engines in the system. The only valid operations on
+/dev/crypto/nx-gzip are:
+
+	* open() the device for read and write.
+	* issue VAS_TX_WIN_OPEN ioctl
+	* mmap() the engine's request queue into application's virtual
+	  address space (i.e. get a paste_address for the co-processor
+	  engine).
+	* close the device node.
+
+Other file operations on this device node are undefined.
+
+Note that the copy and paste operations go directly to the hardware and
+do not go through this device. Refer COPY/PASTE document for more
+details.
+
+Although a system may have several instances of the NX co-processor
+engines (typically, one per P9 chip) there is just one
+/dev/crypto/nx-gzip device node in the system. When the nx-gzip device
+node is opened, Kernel opens send window on a suitable instance of NX
+accelerator. It finds CPU on which the user process is executing and
+determine the NX instance for the corresponding chip on which this CPU
+belongs.
+
+Applications may chose a specific instance of the NX co-processor using
+the vas_id field in the VAS_TX_WIN_OPEN ioctl as detailed below.
+
+A userspace library libnxz is available here but still in development:
+	 https://github.com/abalib/power-gzip
+
+Applications that use inflate / deflate calls can link with libnxz
+instead of libz and use NX GZIP compression without any modification.
+
+Open /dev/crypto/nx-gzip
+========================
+
+The nx-gzip device should be opened for read and write. No special
+privileges are needed to open the device. Each window corresponds to one
+file descriptor. So if the userspace process needs multiple windows,
+several open calls have to be issued.
+
+See open(2) system call man pages for other details such as return values,
+error codes and restrictions.
+
+VAS_TX_WIN_OPEN ioctl
+=====================
+
+Applications should use the VAS_TX_WIN_OPEN ioctl as follows to establish
+a connection with NX co-processor engine:
+
+	::
+		struct vas_tx_win_open_attr {
+			__u32   version;
+			__s16   vas_id; /* specific instance of vas or -1
+						for default */
+			__u16   reserved1;
+			__u64   flags;	/* For future use */
+			__u64   reserved2[6];
+		};
+
+	version: The version field must be currently set to 1.
+	vas_id: If '-1' is passed, kernel will make a best-effort attempt
+		to assign an optimal instance of NX for the process. To
+		select the specific VAS instance, refer
+		"Discovery of available VAS engines" section below.
+
+	flags, reserved1 and reserved2[6] fields are for future extension
+	and must be set to 0.
+
+	The attributes attr for the VAS_TX_WIN_OPEN ioctl are defined as
+	follows:
+		#define VAS_MAGIC 'v'
+		#define VAS_TX_WIN_OPEN _IOW(VAS_MAGIC, 1,
+						struct vas_tx_win_open_attr)
+
+		struct vas_tx_win_open_attr attr;
+		rc = ioctl(fd, VAS_TX_WIN_OPEN, &attr);
+
+	The VAS_TX_WIN_OPEN ioctl returns 0 on success. On errors, it
+	returns -1 and sets the errno variable to indicate the error.
+
+	Error conditions:
+		EINVAL	fd does not refer to a valid VAS device.
+		EINVAL	Invalid vas ID
+		EINVAL	version is not set with proper value
+		EEXIST	Window is already opened for the given fd
+		ENOMEM	Memory is not available to allocate window
+		ENOSPC	System has too many active windows (connections)
+			opened
+		EINVAL	reserved fields are not set to 0.
+
+	See the ioctl(2) man page for more details, error codes and
+	restrictions.
+
+mmap() NX-GZIP device
+=====================
+
+The mmap() system call for a NX-GZIP device fd returns a paste_address
+that the application can use to copy/paste its CRB to the hardware engines.
+	::
+
+		paste_addr = mmap(addr, size, prot, flags, fd, offset);
+
+	Only restrictions on mmap for a NX-GZIP device fd are:
+		* size should be PAGE_SIZE
+		* offset parameter should be 0ULL
+
+	Refer to mmap(2) man page for additional details/restrictions.
+	In addition to the error conditions listed on the mmap(2) man
+	page, can also fail with one of the following error codes:
+
+		EINVAL	fd is not associated with an open window
+			(i.e mmap() does not follow a successful call
+			to the VAS_TX_WIN_OPEN ioctl).
+		EINVAL	offset field is not 0ULL.
+
+Discovery of available VAS engines
+==================================
+
+Each available VAS instance in the system will have a device tree node
+like /proc/device-tree/vas@* or /proc/device-tree/xscom@*/vas@*.
+Determine the chip or VAS instance and use the corresponding ibm,vas-id
+property value in this node to select specific VAS instance.
+
+Copy/Paste operations
+=====================
+
+Applications should use the copy and paste instructions to send CRB to NX.
+Refer section 4.4 in PowerISA for Copy/Paste instructions:
+https://openpowerfoundation.org/?resource_lib=power-isa-version-3-0
+
+CRB Specification and use NX
+============================
+
+Applications should format requests to the co-processor using the
+co-processor Request Block (CRBs). Refer NX-GZIP user's manual for the format
+of CRB and use NX from userspace such as sending requests and checking
+request status.
+
+NX Fault handling
+=================
+
+Applications send requests to NX and wait for the status by polling on
+co-processor Status Block (CSB) flags. NX updates status in CSB after each
+request is processed. Refer NX-GZIP user's manual for the format of CSB and
+status flags.
+
+In case if NX encounters translation error (called NX page fault) on CSB
+address or any request buffer, raises an interrupt on the CPU to handle the
+fault. Page fault can happen if an application passes invalid addresses or
+request buffers are not in memory. The operating system handles the fault by
+updating CSB with the following data:
+
+	csb.flags = CSB_V;
+	csb.cc = CSB_CC_TRANSLATION;
+	csb.ce = CSB_CE_TERMINATION;
+	csb.address = fault_address;
+
+When an application receives translation error, it can touch or access
+the page that has a fault address so that this page will be in memory. Then
+the application can resend this request to NX.
+
+If the OS can not update CSB due to invalid CSB address, sends SEGV signal
+to the process who opened the send window on which the original request was
+issued. This signal returns with the following siginfo struct:
+
+	siginfo.si_signo = SIGSEGV;
+	siginfo.si_errno = EFAULT;
+	siginfo.si_code = SEGV_MAPERR;
+	siginfo.si_addr = CSB adress;
+
+In the case of multi-thread applications, NX send windows can be shared
+across all threads. For example, a child thread can open a send window,
+but other threads can send requests to NX using this window. These
+requests will be successful even in the case of OS handling faults as long
+as CSB address is valid. If the NX request contains an invalid CSB address,
+the signal will be sent to the child thread that opened the window. But if
+the thread is exited without closing the window and the request is issued
+using this window. the signal will be issued to the thread group leader
+(tgid). It is up to the application whether to ignore or handle these
+signals.
+
+NX-GZIP User's Manual:
+https://github.com/libnxz/power-gzip/blob/master/power_nx_gzip_um.pdf
+
+Simple example
+==============
+
+	::
+		int use_nx_gzip()
+		{
+			int rc, fd;
+			void *addr;
+			struct vas_setup_attr txattr;
+
+			fd = open("/dev/crypto/nx-gzip", O_RDWR);
+			if (fd < 0) {
+				fprintf(stderr, "open nx-gzip failed\n");
+				return -1;
+			}
+			memset(&txattr, 0, sizeof(txattr));
+			txattr.version = 1;
+			txattr.vas_id = -1
+			rc = ioctl(fd, VAS_TX_WIN_OPEN,
+					(unsigned long)&txattr);
+			if (rc < 0) {
+				fprintf(stderr, "ioctl() n %d, error %d\n",
+						rc, errno);
+				return rc;
+			}
+			addr = mmap(NULL, 4096, PROT_READ|PROT_WRITE,
+					MAP_SHARED, fd, 0ULL);
+			if (addr == MAP_FAILED) {
+				fprintf(stderr, "mmap() failed, errno %d\n",
+						errno);
+				return -errno;
+			}
+			do {
+				//Format CRB request with compression or
+				//uncompression
+				// Refer tests for vas_copy/vas_paste
+				vas_copy((&crb, 0, 1);
+				vas_paste(addr, 0, 1);
+				// Poll on csb.flags with timeout
+				// csb address is listed in CRB
+			} while (true)
+			close(fd) or window can be closed upon process exit
+		}
+
+	Refer https://github.com/abalib/power-gzip for tests or more
+	use cases.
-- 
1.8.3.1




^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox