[PATCH v3 5/6] plugin: getting qemu_plugin_get_hwaddr only expose one function prototype

Yonggang Luo posted 6 patches 5 years, 4 months ago
Maintainers: Li-Wen Hsu <lwhsu@freebsd.org>, "Alex Bennée" <alex.bennee@linaro.org>, Ed Maste <emaste@freebsd.org>
There is a newer version of this series
[PATCH v3 5/6] plugin: getting qemu_plugin_get_hwaddr only expose one function prototype
Posted by Yonggang Luo 5 years, 4 months ago
This is used for counting how much function are export to qemu plugin.

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
---
 plugins/api.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/plugins/api.c b/plugins/api.c
index f16922ca8b..d325084385 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -252,10 +252,12 @@ bool qemu_plugin_mem_is_store(qemu_plugin_meminfo_t info)
 
 #ifdef CONFIG_SOFTMMU
 static __thread struct qemu_plugin_hwaddr hwaddr_info;
+#endif
 
 struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
                                                   uint64_t vaddr)
 {
+#ifdef CONFIG_SOFTMMU
     CPUState *cpu = current_cpu;
     unsigned int mmu_idx = info >> TRACE_MEM_MMU_SHIFT;
     hwaddr_info.is_store = info & TRACE_MEM_ST;
@@ -267,14 +269,10 @@ struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
     }
 
     return &hwaddr_info;
-}
 #else
-struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
-                                                  uint64_t vaddr)
-{
     return NULL;
-}
 #endif
+}
 
 bool qemu_plugin_hwaddr_is_io(const struct qemu_plugin_hwaddr *haddr)
 {
-- 
2.28.0.windows.1


Re: [PATCH v3 5/6] plugin: getting qemu_plugin_get_hwaddr only expose one function prototype
Posted by Alex Bennée 5 years, 4 months ago
Yonggang Luo <luoyonggang@gmail.com> writes:

> This is used for counting how much function are export to qemu plugin.
>
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> ---
>  plugins/api.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/plugins/api.c b/plugins/api.c
> index f16922ca8b..d325084385 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -252,10 +252,12 @@ bool qemu_plugin_mem_is_store(qemu_plugin_meminfo_t info)
>  
>  #ifdef CONFIG_SOFTMMU
>  static __thread struct qemu_plugin_hwaddr hwaddr_info;
> +#endif
>  
>  struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
>                                                    uint64_t vaddr)
>  {
> +#ifdef CONFIG_SOFTMMU
>      CPUState *cpu = current_cpu;
>      unsigned int mmu_idx = info >> TRACE_MEM_MMU_SHIFT;
>      hwaddr_info.is_store = info & TRACE_MEM_ST;
> @@ -267,14 +269,10 @@ struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
>      }
>  
>      return &hwaddr_info;
> -}
>  #else
> -struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
> -                                                  uint64_t vaddr)
> -{
>      return NULL;
> -}
>  #endif
> +}

Hmm I'm not sure about this, surely you want the plugin system to
complain early if your plugin is going to use a function that is
incorrect for the mode you are running in?

Although we do currently unconditionally export the syscall functions
and arguably they should be CONFIG_USER only as well.

>  
>  bool qemu_plugin_hwaddr_is_io(const struct qemu_plugin_hwaddr *haddr)
>  {


-- 
Alex Bennée

Re: [PATCH v3 5/6] plugin: getting qemu_plugin_get_hwaddr only expose one function prototype
Posted by 罗勇刚 (Yonggang Luo) 5 years, 4 months ago
On Mon, Oct 5, 2020 at 6:48 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> Yonggang Luo <luoyonggang@gmail.com> writes:
>
> > This is used for counting how much function are export to qemu plugin.
> >
> > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> > ---
> >  plugins/api.c | 8 +++-----
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/plugins/api.c b/plugins/api.c
> > index f16922ca8b..d325084385 100644
> > --- a/plugins/api.c
> > +++ b/plugins/api.c
> > @@ -252,10 +252,12 @@ bool
qemu_plugin_mem_is_store(qemu_plugin_meminfo_t info)
> >
> >  #ifdef CONFIG_SOFTMMU
> >  static __thread struct qemu_plugin_hwaddr hwaddr_info;
> > +#endif
> >
> >  struct qemu_plugin_hwaddr
*qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
> >                                                    uint64_t vaddr)
> >  {
> > +#ifdef CONFIG_SOFTMMU
> >      CPUState *cpu = current_cpu;
> >      unsigned int mmu_idx = info >> TRACE_MEM_MMU_SHIFT;
> >      hwaddr_info.is_store = info & TRACE_MEM_ST;
> > @@ -267,14 +269,10 @@ struct qemu_plugin_hwaddr
*qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
> >      }
> >
> >      return &hwaddr_info;
> > -}
> >  #else
> > -struct qemu_plugin_hwaddr
*qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
> > -                                                  uint64_t vaddr)
> > -{
> >      return NULL;
> > -}
> >  #endif
> > +}
>
> Hmm I'm not sure about this, surely you want the plugin system to
> complain early if your plugin is going to use a function that is
> incorrect for the mode you are running in?
I merged these two function for couting how much function are exported, so
getting the code easier to review, otherwise
 function qemu_plugin_get_hwaddr   would be exported twice.
>
> Although we do currently unconditionally export the syscall functions
> and arguably they should be CONFIG_USER only as well.
>
> >
> >  bool qemu_plugin_hwaddr_is_io(const struct qemu_plugin_hwaddr *haddr)
> >  {
>
>
> --
> Alex Bennée



--
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo
Re: [PATCH v3 5/6] plugin: getting qemu_plugin_get_hwaddr only expose one function prototype
Posted by Alex Bennée 5 years, 4 months ago
罗勇刚(Yonggang Luo) <luoyonggang@gmail.com> writes:

> On Mon, Oct 5, 2020 at 6:48 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>>
>> Yonggang Luo <luoyonggang@gmail.com> writes:
>>
>> > This is used for counting how much function are export to qemu plugin.
>> >
>> > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
>> > ---
>> >  plugins/api.c | 8 +++-----
>> >  1 file changed, 3 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/plugins/api.c b/plugins/api.c
>> > index f16922ca8b..d325084385 100644
>> > --- a/plugins/api.c
>> > +++ b/plugins/api.c
>> > @@ -252,10 +252,12 @@ bool
> qemu_plugin_mem_is_store(qemu_plugin_meminfo_t info)
>> >
>> >  #ifdef CONFIG_SOFTMMU
>> >  static __thread struct qemu_plugin_hwaddr hwaddr_info;
>> > +#endif
>> >
>> >  struct qemu_plugin_hwaddr
> *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
>> >                                                    uint64_t vaddr)
>> >  {
>> > +#ifdef CONFIG_SOFTMMU
>> >      CPUState *cpu = current_cpu;
>> >      unsigned int mmu_idx = info >> TRACE_MEM_MMU_SHIFT;
>> >      hwaddr_info.is_store = info & TRACE_MEM_ST;
>> > @@ -267,14 +269,10 @@ struct qemu_plugin_hwaddr
> *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
>> >      }
>> >
>> >      return &hwaddr_info;
>> > -}
>> >  #else
>> > -struct qemu_plugin_hwaddr
> *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
>> > -                                                  uint64_t vaddr)
>> > -{
>> >      return NULL;
>> > -}
>> >  #endif
>> > +}
>>
>> Hmm I'm not sure about this, surely you want the plugin system to
>> complain early if your plugin is going to use a function that is
>> incorrect for the mode you are running in?
> I merged these two function for couting how much function are exported, so
> getting the code easier to review, otherwise
>  function qemu_plugin_get_hwaddr   would be exported twice.

Ahh I see now..

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée