Re: [PATCH 2/4] hw/i386/sgx: Have sgx_epc_get_section() return a boolean

[Yang Zhong <yang.zhong@intel.com>]

On Thu, Oct 07, 2021 at 07:56:10PM +0200, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  include/hw/i386/sgx-epc.h | 2 +-
>  hw/i386/sgx-stub.c        | 2 +-
>  hw/i386/sgx.c             | 6 +++---
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/hw/i386/sgx-epc.h b/include/hw/i386/sgx-epc.h
> index 65a68ca753a..a6a65be854f 100644
> --- a/include/hw/i386/sgx-epc.h
> +++ b/include/hw/i386/sgx-epc.h
> @@ -55,7 +55,7 @@ typedef struct SGXEPCState {
>      int nr_sections;
>  } SGXEPCState;
>  
> -int sgx_epc_get_section(int section_nr, uint64_t *addr, uint64_t *size);
> +bool sgx_epc_get_section(int section_nr, uint64_t *addr, uint64_t *size);
>  
>  static inline uint64_t sgx_epc_above_4g_end(SGXEPCState *sgx_epc)
>  {
> diff --git a/hw/i386/sgx-stub.c b/hw/i386/sgx-stub.c
> index 3be9f5ca32c..45c473119ef 100644
> --- a/hw/i386/sgx-stub.c
> +++ b/hw/i386/sgx-stub.c
> @@ -20,7 +20,7 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms)
>      memset(&pcms->sgx_epc, 0, sizeof(SGXEPCState));
>  }
>  
> -int sgx_epc_get_section(int section_nr, uint64_t *addr, uint64_t *size)
> +bool sgx_epc_get_section(int section_nr, uint64_t *addr, uint64_t *size)
>  {
>      g_assert_not_reached();
>  }
> diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c
> index e481e9358f1..29724ff8f08 100644
> --- a/hw/i386/sgx.c
> +++ b/hw/i386/sgx.c
> @@ -115,13 +115,13 @@ SGXInfo *sgx_get_info(Error **errp)
>      return info;
>  }
>  
> -int sgx_epc_get_section(int section_nr, uint64_t *addr, uint64_t *size)
> +bool sgx_epc_get_section(int section_nr, uint64_t *addr, uint64_t *size)
>  {
>      PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
>      SGXEPCDevice *epc;
>  
>      if (pcms->sgx_epc.size == 0 || pcms->sgx_epc.nr_sections <= section_nr) {
> -        return 1;
> +        return true;


If return boolean, here should be return false, Sean wrote this(return 0 or 1) like Linux kernel did.  


>      }
>  
>      epc = pcms->sgx_epc.sections[section_nr];
> @@ -129,7 +129,7 @@ int sgx_epc_get_section(int section_nr, uint64_t *addr, uint64_t *size)
>      *addr = epc->addr;
>      *size = memory_device_get_region_size(MEMORY_DEVICE(epc), &error_fatal);
>  
> -    return 0;
> +    return false;

Here should be return true.

Then in the ./target/i386/cpu.c file,

 if (sgx_epc_get_section(count - 2, &epc_addr, &epc_size))

 should be

 if (!sgx_epc_get_section(count - 2, &epc_addr, &epc_size))


Yang


>  }
>  
>  void pc_machine_init_sgx_epc(PCMachineState *pcms)
> -- 
> 2.31.1