Re: [PATCH 1/2] KVM: kvm_is_visible_gfn can be boolean

[Amos Jianjun Kong]

On Sat, Nov 14, 2015 at 11:21 AM, Yaowei Bai
 wrote:
> This patch makes kvm_is_visible_gfn return bool due to this particular
> function only using either one or zero as its return value.
>
> No functional change.
>
> Signed-off-by: Yaowei Bai 

Hi Yaowei,

> ---
>  include/linux/kvm_host.h | 2 +-
>  virt/kvm/kvm_main.c  | 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 5706a21..4436539 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -623,7 +623,7 @@ int kvm_gfn_to_hva_cache_init(struct kvm *kvm, struct 
> gfn_to_hva_cache *ghc,
>  int kvm_clear_guest_page(struct kvm *kvm, gfn_t gfn, int offset, int len);
>  int kvm_clear_guest(struct kvm *kvm, gpa_t gpa, unsigned long len);
>  struct kvm_memory_slot *gfn_to_memslot(struct kvm *kvm, gfn_t gfn);
> -int kvm_is_visible_gfn(struct kvm *kvm, gfn_t gfn);
> +bool kvm_is_visible_gfn(struct kvm *kvm, gfn_t gfn);
>  unsigned long kvm_host_page_size(struct kvm *kvm, gfn_t gfn);
>  void mark_page_dirty(struct kvm *kvm, gfn_t gfn);
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 484079e..73cbb41 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -1164,15 +1164,15 @@ struct kvm_memory_slot 
> *kvm_vcpu_gfn_to_memslot(struct kvm_vcpu *vcpu, gfn_t gfn
> return __gfn_to_memslot(kvm_vcpu_memslots(vcpu), gfn);
>  }
>
> -int kvm_is_visible_gfn(struct kvm *kvm, gfn_t gfn)
> +bool kvm_is_visible_gfn(struct kvm *kvm, gfn_t gfn)
>  {
> struct kvm_memory_slot *memslot = gfn_to_memslot(kvm, gfn);
>
> if (!memslot || memslot->id >= KVM_USER_MEM_SLOTS ||
>   memslot->flags & KVM_MEMSLOT_INVALID)
> -   return 0;
> +   return false;
>
> -   return 1;
> +   return true;
>  }
>  EXPORT_SYMBOL_GPL(kvm_is_visible_gfn);


kvm_is_visible_gfn() is also used in arch/powerpc/kvm/book3s_pr.c:

static int kvmppc_visible_gpa(struct kvm_vcpu *vcpu, gpa_t gpa)
{
..
if (unlikely(mp_pa) && unlikely((mp_pa & KVM_PAM) == (gpa & KVM_PAM))) {
return 1;
}

return kvm_is_visible_gfn(vcpu->kvm, gpa >> PAGE_SHIFT);
}

Do we still need to update that function?

Thanks, Amos
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/2] KVM: kvm_is_visible_gfn can be boolean

[Amos Jianjun Kong]

On Sat, Nov 14, 2015 at 11:21 AM, Yaowei Bai
 wrote:
> This patch makes kvm_is_visible_gfn return bool due to this particular
> function only using either one or zero as its return value.
>
> No functional change.
>
> Signed-off-by: Yaowei Bai 

Hi Yaowei,

> ---
>  include/linux/kvm_host.h | 2 +-
>  virt/kvm/kvm_main.c  | 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 5706a21..4436539 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -623,7 +623,7 @@ int kvm_gfn_to_hva_cache_init(struct kvm *kvm, struct 
> gfn_to_hva_cache *ghc,
>  int kvm_clear_guest_page(struct kvm *kvm, gfn_t gfn, int offset, int len);
>  int kvm_clear_guest(struct kvm *kvm, gpa_t gpa, unsigned long len);
>  struct kvm_memory_slot *gfn_to_memslot(struct kvm *kvm, gfn_t gfn);
> -int kvm_is_visible_gfn(struct kvm *kvm, gfn_t gfn);
> +bool kvm_is_visible_gfn(struct kvm *kvm, gfn_t gfn);
>  unsigned long kvm_host_page_size(struct kvm *kvm, gfn_t gfn);
>  void mark_page_dirty(struct kvm *kvm, gfn_t gfn);
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 484079e..73cbb41 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -1164,15 +1164,15 @@ struct kvm_memory_slot 
> *kvm_vcpu_gfn_to_memslot(struct kvm_vcpu *vcpu, gfn_t gfn
> return __gfn_to_memslot(kvm_vcpu_memslots(vcpu), gfn);
>  }
>
> -int kvm_is_visible_gfn(struct kvm *kvm, gfn_t gfn)
> +bool kvm_is_visible_gfn(struct kvm *kvm, gfn_t gfn)
>  {
> struct kvm_memory_slot *memslot = gfn_to_memslot(kvm, gfn);
>
> if (!memslot || memslot->id >= KVM_USER_MEM_SLOTS ||
>   memslot->flags & KVM_MEMSLOT_INVALID)
> -   return 0;
> +   return false;
>
> -   return 1;
> +   return true;
>  }
>  EXPORT_SYMBOL_GPL(kvm_is_visible_gfn);


kvm_is_visible_gfn() is also used in arch/powerpc/kvm/book3s_pr.c:

static int kvmppc_visible_gpa(struct kvm_vcpu *vcpu, gpa_t gpa)
{
..
if (unlikely(mp_pa) && unlikely((mp_pa & KVM_PAM) == (gpa & KVM_PAM))) {
return 1;
}

return kvm_is_visible_gfn(vcpu->kvm, gpa >> PAGE_SHIFT);
}

Do we still need to update that function?

Thanks, Amos
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] gpio / ACPI: Return -EPROBE_DEFER if the gpiochip was not found

[Amos Jianjun Kong]

On Wed, Jun 10, 2015 at 9:05 PM, Mika Westerberg
 wrote:
> If a driver requests a GPIO described in its _CRS but the GPIO host
> controller (gpiochip) driver providing the GPIO has not been loaded yet
> acpi_get_gpiod() returns -ENODEV which causes the calling driver to fail.
>
> If the gpiochip driver is loaded afterwards the driver requesting the GPIO
> will not notice this.
>
> Better approach is to return -EPROBE_DEFER in such case. Then when the
> gpiochip driver appears the driver requesting the GPIO will be probed
> again. This also aligns ACPI GPIO lookup code closer to DT as it does
> pretty much the same when no gpiochip driver was found.
>
> Reported-by: Tobias Diedrich 
> Signed-off-by: Mika Westerberg 
> ---
>  drivers/gpio/gpiolib-acpi.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> index 19b99d0c2bf0..b49006c81a7f 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> @@ -114,10 +114,11 @@ static inline int 
> acpi_gpiochip_pin_to_gpio_offset(struct gpio_chip *chip,
>   * @path:  ACPI GPIO controller full path name, (e.g. "\\_SB.GPO1")
>   * @pin:   ACPI GPIO pin number (0-based, controller-relative)
>   *
> - * Returns GPIO descriptor to use with Linux generic GPIO API, or ERR_PTR
> - * error value
> + * Return: GPIO descriptor to use with Linux generic GPIO API, or ERR_PTR
> + * error value. Specifically returns %-EPROBE_DEFER if the referenced GPIO
> + * controller does not have gpiochip registered at the moment. This is to
> + * support probe deferral.
>   */
> -

api_get_gpiod() is called in preprobe stage, we still have chance to
probe the device.

Reviewed-by: Amos Kong 

>  static struct gpio_desc *acpi_get_gpiod(char *path, int pin)
>  {
> struct gpio_chip *chip;
> @@ -131,7 +132,7 @@ static struct gpio_desc *acpi_get_gpiod(char *path, int 
> pin)
>
> chip = gpiochip_find(handle, acpi_gpiochip_find);
> if (!chip)
> -   return ERR_PTR(-ENODEV);
> +   return ERR_PTR(-EPROBE_DEFER);
>
> offset = acpi_gpiochip_pin_to_gpio_offset(chip, pin);
> if (offset < 0)
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] gpio / ACPI: Return -EPROBE_DEFER if the gpiochip was not found

[Amos Jianjun Kong]

On Wed, Jun 10, 2015 at 9:05 PM, Mika Westerberg
mika.westerb...@linux.intel.com wrote:
 If a driver requests a GPIO described in its _CRS but the GPIO host
 controller (gpiochip) driver providing the GPIO has not been loaded yet
 acpi_get_gpiod() returns -ENODEV which causes the calling driver to fail.

 If the gpiochip driver is loaded afterwards the driver requesting the GPIO
 will not notice this.

 Better approach is to return -EPROBE_DEFER in such case. Then when the
 gpiochip driver appears the driver requesting the GPIO will be probed
 again. This also aligns ACPI GPIO lookup code closer to DT as it does
 pretty much the same when no gpiochip driver was found.

 Reported-by: Tobias Diedrich tobiasdiedr...@gmail.com
 Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
 ---
  drivers/gpio/gpiolib-acpi.c | 9 +
  1 file changed, 5 insertions(+), 4 deletions(-)

 diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
 index 19b99d0c2bf0..b49006c81a7f 100644
 --- a/drivers/gpio/gpiolib-acpi.c
 +++ b/drivers/gpio/gpiolib-acpi.c
 @@ -114,10 +114,11 @@ static inline int 
 acpi_gpiochip_pin_to_gpio_offset(struct gpio_chip *chip,
   * @path:  ACPI GPIO controller full path name, (e.g. \\_SB.GPO1)
   * @pin:   ACPI GPIO pin number (0-based, controller-relative)
   *
 - * Returns GPIO descriptor to use with Linux generic GPIO API, or ERR_PTR
 - * error value
 + * Return: GPIO descriptor to use with Linux generic GPIO API, or ERR_PTR
 + * error value. Specifically returns %-EPROBE_DEFER if the referenced GPIO
 + * controller does not have gpiochip registered at the moment. This is to
 + * support probe deferral.
   */
 -

api_get_gpiod() is called in preprobe stage, we still have chance to
probe the device.

Reviewed-by: Amos Kong kongjian...@gmail.com

  static struct gpio_desc *acpi_get_gpiod(char *path, int pin)
  {
 struct gpio_chip *chip;
 @@ -131,7 +132,7 @@ static struct gpio_desc *acpi_get_gpiod(char *path, int 
 pin)

 chip = gpiochip_find(handle, acpi_gpiochip_find);
 if (!chip)
 -   return ERR_PTR(-ENODEV);
 +   return ERR_PTR(-EPROBE_DEFER);

 offset = acpi_gpiochip_pin_to_gpio_offset(chip, pin);
 if (offset  0)
 --
 2.1.4

 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/