[Skiboot] [PATCH 03/12] opal-api: add endian conversions to most opal calls

Nicholas Piggin npiggin at gmail.com
Wed Oct 2 13:29:44 AEST 2019


Oliver O'Halloran's on October 1, 2019 1:14 pm:
> On Sun, 2019-09-29 at 17:46 +1000, Nicholas Piggin wrote:
>> This adds missing endian conversions to most calls, sufficient at least
>> to handle calls from a kernel booting on mambo.
>> 
>> Subsystems requiring more extensive changes (e.g., xive) will be done
>> with individual changes.
>> 
>> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
>> ---
>>  core/console.c          | 19 ++++++----
>>  core/interrupts.c       |  8 +++--
>>  core/ipmi-opal.c        |  6 ++--
>>  core/pci-opal.c         | 80 +++++++++++++++++++++++++++++++++--------
>>  core/powercap.c         | 12 +++++--
>>  core/psr.c              | 12 +++++--
>>  core/sensor.c           | 32 +++++++++++------
>>  hw/fake-rtc.c           |  9 +++--
>>  hw/fsp/fsp-console.c    | 26 ++++++++------
>>  hw/fsp/fsp-rtc.c        | 20 ++++++-----
>>  hw/ipmi/ipmi-rtc.c      |  6 +++-
>>  hw/lpc-rtc.c            |  6 +++-
>>  hw/lpc-uart.c           | 16 ++++-----
>>  hw/npu2-opencapi.c      | 10 ++++--
>>  hw/xscom.c              | 19 ++++++++--
>>  include/console.h       |  6 ++--
>>  platforms/mambo/mambo.c |  7 +++-
>>  17 files changed, 213 insertions(+), 81 deletions(-)
>> 
>> diff --git a/core/console.c b/core/console.c
>> index 139ba4a97..ac88f0c71 100644
>> --- a/core/console.c
>> +++ b/core/console.c
>> @@ -351,22 +351,25 @@ void memcons_add_properties(void)
>>   * complicated since they can come from the in-memory console (BML) or from the
>>   * internal skiboot console driver.
>>   */
>> -static int64_t dummy_console_write(int64_t term_number, int64_t *length,
>> +static int64_t dummy_console_write(int64_t term_number, __be64 *length,
>>  				   const uint8_t *buffer)
>>  {
>> +	uint64_t l;
>> +
>>  	if (term_number != 0)
>>  		return OPAL_PARAMETER;
>>  
>>  	if (!opal_addr_valid(length) || !opal_addr_valid(buffer))
>>  		return OPAL_PARAMETER;
>>  
>> -	write(0, buffer, *length);
>> +	l = be64_to_cpu(*length);
>> +	write(0, buffer, l);
>>  
>>  	return OPAL_SUCCESS;
>>  }
>>  
>>  static int64_t dummy_console_write_buffer_space(int64_t term_number,
>> -						int64_t *length)
>> +						__be64 *length)
>>  {
>>  	if (term_number != 0)
>>  		return OPAL_PARAMETER;
>> @@ -375,21 +378,25 @@ static int64_t dummy_console_write_buffer_space(int64_t term_number,
>>  		return OPAL_PARAMETER;
>>  
>>  	if (length)
>> -		*length = INMEM_CON_OUT_LEN;
>> +		*length = cpu_to_be64(INMEM_CON_OUT_LEN);
>>  
>>  	return OPAL_SUCCESS;
>>  }
>>  
>> -static int64_t dummy_console_read(int64_t term_number, int64_t *length,
>> +static int64_t dummy_console_read(int64_t term_number, __be64 *length,
>>  				  uint8_t *buffer)
>>  {
>> +	uint64_t l;
>> +
>>  	if (term_number != 0)
>>  		return OPAL_PARAMETER;
>>  
>>  	if (!opal_addr_valid(length) || !opal_addr_valid(buffer))
>>  		return OPAL_PARAMETER;
>>  
>> -	*length = read(0, buffer, *length);
>> +	l = be64_to_cpu(*length);
>> +	l = read(0, buffer, l);
>> +	*length = cpu_to_be64(l);
>>  	opal_update_pending_evt(OPAL_EVENT_CONSOLE_INPUT, 0);
>>  
>>  	return OPAL_SUCCESS;
>> diff --git a/core/interrupts.c b/core/interrupts.c
>> index 10baa15f6..d4a2c3124 100644
>> --- a/core/interrupts.c
>> +++ b/core/interrupts.c
>> @@ -439,9 +439,11 @@ static int64_t opal_set_xive(uint32_t isn, uint16_t server, uint8_t priority)
>>  }
>>  opal_call(OPAL_SET_XIVE, opal_set_xive, 3);
>>  
>> -static int64_t opal_get_xive(uint32_t isn, uint16_t *server, uint8_t *priority)
>> +static int64_t opal_get_xive(uint32_t isn, __be16 *server, uint8_t *priority)
>>  {
>>  	struct irq_source *is = irq_find_source(isn);
>> +	uint16_t s;
>> +	int64_t ret;
>>  
>>  	if (!opal_addr_valid(server))
>>  		return OPAL_PARAMETER;
>> @@ -449,7 +451,9 @@ static int64_t opal_get_xive(uint32_t isn, uint16_t *server, uint8_t *priority)
>>  	if (!is || !is->ops->get_xive)
>>  		return OPAL_PARAMETER;
>>  
>> -	return is->ops->get_xive(is, isn, server, priority);
>> +	ret = is->ops->get_xive(is, isn, &s, priority);
>> +	*server = cpu_to_be16(s);
>> +	return ret;
>>  }
>>  opal_call(OPAL_GET_XIVE, opal_get_xive, 3);
>>  
>> diff --git a/core/ipmi-opal.c b/core/ipmi-opal.c
>> index 796508ca0..d36962d36 100644
>> --- a/core/ipmi-opal.c
>> +++ b/core/ipmi-opal.c
>> @@ -57,7 +57,7 @@ static int64_t opal_ipmi_send(uint64_t interface,
>>  }
>>  
>>  static int64_t opal_ipmi_recv(uint64_t interface,
>> -			      struct opal_ipmi_msg *opal_ipmi_msg, uint64_t *msg_len)
>> +			      struct opal_ipmi_msg *opal_ipmi_msg, __be64 *msg_len)
>>  {
>>  	struct ipmi_msg *msg;
>>  	int64_t rc;
>> @@ -82,7 +82,7 @@ static int64_t opal_ipmi_recv(uint64_t interface,
>>  		goto out_del_msg;
>>  	}
>>  
>> -	if (*msg_len - sizeof(struct opal_ipmi_msg) < msg->resp_size + 1) {
>> +	if (be64_to_cpu(*msg_len) - sizeof(struct opal_ipmi_msg) < msg->resp_size + 1) {
>>  		rc = OPAL_RESOURCE;
>>  		goto out_del_msg;
>>  	}
>> @@ -101,7 +101,7 @@ static int64_t opal_ipmi_recv(uint64_t interface,
>>  	      msg->cmd, msg->netfn >> 2, msg->resp_size);
>>  
>>  	/* Add one as the completion code is returned in the message data */
>> -	*msg_len = msg->resp_size + sizeof(struct opal_ipmi_msg) + 1;
>> +	*msg_len = cpu_to_be64(msg->resp_size + sizeof(struct opal_ipmi_msg) + 1);
>>  	ipmi_free_msg(msg);
>>  
>>  	return OPAL_SUCCESS;
>> diff --git a/core/pci-opal.c b/core/pci-opal.c
>> index 213a72565..e0ab3cf91 100644
>> --- a/core/pci-opal.c
>> +++ b/core/pci-opal.c
>> @@ -58,9 +58,36 @@ OPAL_PCICFG_ACCESS_WRITE(write_byte,		write8, uint8_t)
>>  OPAL_PCICFG_ACCESS_WRITE(write_half_word,	write16, uint16_t)
>>  OPAL_PCICFG_ACCESS_WRITE(write_word,		write32, uint32_t)
>>  
>> +static int64_t opal_pci_config_read_half_word_be(uint64_t phb_id,
>> +						 uint64_t bus_dev_func,
>> +						 uint64_t offset,
>> +						 __be16 *data)
>> +{
>> +	uint16_t __data;
>> +	int64_t rc;
>> +
>> +	rc = opal_pci_config_read_half_word(phb_id, bus_dev_func, offset, &__data);
>> +	*data = cpu_to_be16(__data);
>> +	return rc;
>> +}
>> +
>> +static int64_t opal_pci_config_read_word_be(uint64_t phb_id,
>> +						 uint64_t bus_dev_func,
>> +						 uint64_t offset,
>> +						 __be32 *data)
>> +{
>> +	uint32_t __data;
>> +	int64_t rc;
>> +
>> +	rc = opal_pci_config_read_word(phb_id, bus_dev_func, offset, &__data);
>> +	*data = cpu_to_be32(__data);
>> +	return rc;
>> +}
>> +
>> +
>>  opal_call(OPAL_PCI_CONFIG_READ_BYTE, opal_pci_config_read_byte, 4);
>> -opal_call(OPAL_PCI_CONFIG_READ_HALF_WORD, opal_pci_config_read_half_word, 4);
>> -opal_call(OPAL_PCI_CONFIG_READ_WORD, opal_pci_config_read_word, 4);
>> +opal_call(OPAL_PCI_CONFIG_READ_HALF_WORD, opal_pci_config_read_half_word_be, 4);
>> +opal_call(OPAL_PCI_CONFIG_READ_WORD, opal_pci_config_read_word_be, 4);
>>  opal_call(OPAL_PCI_CONFIG_WRITE_BYTE, opal_pci_config_write_byte, 4);
>>  opal_call(OPAL_PCI_CONFIG_WRITE_HALF_WORD, opal_pci_config_write_half_word, 4);
>>  opal_call(OPAL_PCI_CONFIG_WRITE_WORD, opal_pci_config_write_word, 4);
>> @@ -87,10 +114,11 @@ void opal_pci_eeh_clear_evt(uint64_t phb_id)
>>  
>>  static int64_t opal_pci_eeh_freeze_status(uint64_t phb_id, uint64_t pe_number,
>>  					  uint8_t *freeze_state,
>> -					  uint16_t *pci_error_type,
>> -					  uint64_t *phb_status)
>> +					  __be16 *pci_error_type,
>> +					  __be64 *phb_status)
>>  {
>>  	struct phb *phb = pci_get_phb(phb_id);
>> +	uint16_t __pci_error_type;
>>  	int64_t rc;
>>  
>>  	if (!opal_addr_valid(freeze_state) || !opal_addr_valid(pci_error_type)
>> @@ -108,7 +136,8 @@ static int64_t opal_pci_eeh_freeze_status(uint64_t phb_id, uint64_t pe_number,
>>  				phb_id, __func__);
>>  
>>  	rc = phb->ops->eeh_freeze_status(phb, pe_number, freeze_state,
>> -					 pci_error_type, NULL);
>> +					 &__pci_error_type, NULL);
>> +	*pci_error_type = cpu_to_be16(__pci_error_type);
>>  	phb_unlock(phb);
>>  
>>  	return rc;
>> @@ -371,9 +400,11 @@ opal_call(OPAL_PCI_SET_XIVE_PE, opal_pci_set_xive_pe, 3);
>>  
>>  static int64_t opal_get_msi_32(uint64_t phb_id, uint32_t mve_number,
>>  			       uint32_t xive_num, uint8_t msi_range,
>> -			       uint32_t *msi_address, uint32_t *message_data)
>> +			       __be32 *msi_address, __be32 *message_data)
>>  {
>>  	struct phb *phb = pci_get_phb(phb_id);
>> +	uint32_t __msi_address;
>> +	uint32_t __message_data;
> 
> I'd rather you kept it consistent and used the __ prefix for the BE
> output variables rather than the native endian ones. Makes for a less

Whatever you prefer. My thought process was the non-prefixed is the
acutal OPAL API format, but that may actually be more confusing. Your
call so I'll change it.

Thanks,
Nick


More information about the Skiboot mailing list