| Message ID | 20181024011909.21674-1-s-anna@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | rpmsg: virtio_rpmsg_bus: replace "%p" with "%pK" | expand |
Hi Bjorn, On 10/23/18 8:19 PM, Suman Anna wrote: > The virtio_rpmsg_bus driver uses the "%p" format-specifier for > printing the vring buffer address. This prints only a hashed > pointer even for previliged users. Use "%pK" instead so that > the address can be printed during debug using kptr_restrict > sysctl. Seems to have been lost among the patches, can you pick up this trivial patch for 5.4? Should apply cleanly on the latest HEAD as well. regards Suman > > Signed-off-by: Suman Anna <s-anna@ti.com> > --- > drivers/rpmsg/virtio_rpmsg_bus.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c > index f29dee731026..1345f373a1a0 100644 > --- a/drivers/rpmsg/virtio_rpmsg_bus.c > +++ b/drivers/rpmsg/virtio_rpmsg_bus.c > @@ -950,7 +950,7 @@ static int rpmsg_probe(struct virtio_device *vdev) > goto vqs_del; > } > > - dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n", > + dev_dbg(&vdev->dev, "buffers: va %pK, dma %pad\n", > bufs_va, &vrp->bufs_dma); > > /* half of the buffers is dedicated for RX */ >
On 10/23/18 9:19 PM, Suman Anna wrote: > The virtio_rpmsg_bus driver uses the "%p" format-specifier for > printing the vring buffer address. This prints only a hashed > pointer even for previliged users. Use "%pK" instead so that > the address can be printed during debug using kptr_restrict > sysctl. > s/previliged/privileged You describe what the code does, but not why you need this. %pK is used for only about 1% of pointer printing, why do you want to leak this address to userspace at all? Andrew > Signed-off-by: Suman Anna <s-anna@ti.com> > --- > drivers/rpmsg/virtio_rpmsg_bus.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c > index f29dee731026..1345f373a1a0 100644 > --- a/drivers/rpmsg/virtio_rpmsg_bus.c > +++ b/drivers/rpmsg/virtio_rpmsg_bus.c > @@ -950,7 +950,7 @@ static int rpmsg_probe(struct virtio_device *vdev) > goto vqs_del; > } > > - dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n", > + dev_dbg(&vdev->dev, "buffers: va %pK, dma %pad\n", > bufs_va, &vrp->bufs_dma); > > /* half of the buffers is dedicated for RX */ >
On 8/12/19 10:47 AM, Andrew F. Davis wrote: > On 10/23/18 9:19 PM, Suman Anna wrote: >> The virtio_rpmsg_bus driver uses the "%p" format-specifier for >> printing the vring buffer address. This prints only a hashed >> pointer even for previliged users. Use "%pK" instead so that >> the address can be printed during debug using kptr_restrict >> sysctl. >> > > > s/previliged/privileged Bjorn, Can you fix this up when applying. > > You describe what the code does, but not why you need this. %pK is used > for only about 1% of pointer printing, why do you want to leak this > address to userspace at all? Andrew, Default behavior of %pK is same as %p, but it does allow you to control the print. The reason is clearly mentioned in the last sentence in the patch description. regards Suman > > Andrew > > >> Signed-off-by: Suman Anna <s-anna@ti.com> >> --- >> drivers/rpmsg/virtio_rpmsg_bus.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c >> index f29dee731026..1345f373a1a0 100644 >> --- a/drivers/rpmsg/virtio_rpmsg_bus.c >> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c >> @@ -950,7 +950,7 @@ static int rpmsg_probe(struct virtio_device *vdev) >> goto vqs_del; >> } >> >> - dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n", >> + dev_dbg(&vdev->dev, "buffers: va %pK, dma %pad\n", >> bufs_va, &vrp->bufs_dma); >> >> /* half of the buffers is dedicated for RX */ >>
On 8/12/19 12:28 PM, Suman Anna wrote: > On 8/12/19 10:47 AM, Andrew F. Davis wrote: >> On 10/23/18 9:19 PM, Suman Anna wrote: >>> The virtio_rpmsg_bus driver uses the "%p" format-specifier for >>> printing the vring buffer address. This prints only a hashed >>> pointer even for previliged users. Use "%pK" instead so that >>> the address can be printed during debug using kptr_restrict >>> sysctl. >>> >> >> >> s/previliged/privileged > > Bjorn, > Can you fix this up when applying. > >> >> You describe what the code does, but not why you need this. %pK is used >> for only about 1% of pointer printing, why do you want to leak this >> address to userspace at all? > > Andrew, > Default behavior of %pK is same as %p, but it does allow you to control > the print. The reason is clearly mentioned in the last sentence in the > patch description. > Let me rephrase then, why would you ever set 'kptr_restrict' to anything other than 0, or why do you want to be able to leak this address to userspace at all? If the answer is just because you can, then all 12,000 instances of %p in kernel could be converted for the same reason. Andrew > regards > Suman > >> >> Andrew >> >> >>> Signed-off-by: Suman Anna <s-anna@ti.com> >>> --- >>> drivers/rpmsg/virtio_rpmsg_bus.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c >>> index f29dee731026..1345f373a1a0 100644 >>> --- a/drivers/rpmsg/virtio_rpmsg_bus.c >>> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c >>> @@ -950,7 +950,7 @@ static int rpmsg_probe(struct virtio_device *vdev) >>> goto vqs_del; >>> } >>> >>> - dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n", >>> + dev_dbg(&vdev->dev, "buffers: va %pK, dma %pad\n", >>> bufs_va, &vrp->bufs_dma); >>> >>> /* half of the buffers is dedicated for RX */ >>> >
On 8/12/19 11:36 AM, Andrew F. Davis wrote: > On 8/12/19 12:28 PM, Suman Anna wrote: >> On 8/12/19 10:47 AM, Andrew F. Davis wrote: >>> On 10/23/18 9:19 PM, Suman Anna wrote: >>>> The virtio_rpmsg_bus driver uses the "%p" format-specifier for >>>> printing the vring buffer address. This prints only a hashed >>>> pointer even for previliged users. Use "%pK" instead so that >>>> the address can be printed during debug using kptr_restrict >>>> sysctl. >>>> >>> >>> >>> s/previliged/privileged >> >> Bjorn, >> Can you fix this up when applying. >> >>> >>> You describe what the code does, but not why you need this. %pK is used >>> for only about 1% of pointer printing, why do you want to leak this >>> address to userspace at all? >> >> Andrew, >> Default behavior of %pK is same as %p, but it does allow you to control >> the print. The reason is clearly mentioned in the last sentence in the >> patch description. >> > > > Let me rephrase then, why would you ever set 'kptr_restrict' to anything > other than 0, or why do you want to be able to leak this address to > userspace at all? If the answer is just because you can, then all 12,000 > instances of %p in kernel could be converted for the same reason. It is a dev_dbg statement, so it is already under dynamic debug control. We would only ever use it during debug. regards Suman > > Andrew > > >> regards >> Suman >> >>> >>> Andrew >>> >>> >>>> Signed-off-by: Suman Anna <s-anna@ti.com> >>>> --- >>>> drivers/rpmsg/virtio_rpmsg_bus.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c >>>> index f29dee731026..1345f373a1a0 100644 >>>> --- a/drivers/rpmsg/virtio_rpmsg_bus.c >>>> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c >>>> @@ -950,7 +950,7 @@ static int rpmsg_probe(struct virtio_device *vdev) >>>> goto vqs_del; >>>> } >>>> >>>> - dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n", >>>> + dev_dbg(&vdev->dev, "buffers: va %pK, dma %pad\n", >>>> bufs_va, &vrp->bufs_dma); >>>> >>>> /* half of the buffers is dedicated for RX */ >>>> >>
On 8/12/19 12:39 PM, Suman Anna wrote: > On 8/12/19 11:36 AM, Andrew F. Davis wrote: >> On 8/12/19 12:28 PM, Suman Anna wrote: >>> On 8/12/19 10:47 AM, Andrew F. Davis wrote: >>>> On 10/23/18 9:19 PM, Suman Anna wrote: >>>>> The virtio_rpmsg_bus driver uses the "%p" format-specifier for >>>>> printing the vring buffer address. This prints only a hashed >>>>> pointer even for previliged users. Use "%pK" instead so that >>>>> the address can be printed during debug using kptr_restrict >>>>> sysctl. >>>>> >>>> >>>> >>>> s/previliged/privileged >>> >>> Bjorn, >>> Can you fix this up when applying. >>> >>>> >>>> You describe what the code does, but not why you need this. %pK is used >>>> for only about 1% of pointer printing, why do you want to leak this >>>> address to userspace at all? >>> >>> Andrew, >>> Default behavior of %pK is same as %p, but it does allow you to control >>> the print. The reason is clearly mentioned in the last sentence in the >>> patch description. >>> >> >> >> Let me rephrase then, why would you ever set 'kptr_restrict' to anything >> other than 0, or why do you want to be able to leak this address to >> userspace at all? If the answer is just because you can, then all 12,000 >> instances of %p in kernel could be converted for the same reason. > > It is a dev_dbg statement, so it is already under dynamic debug control. > We would only ever use it during debug. > Most pointer printings are in debug statements.. I'm simply not seeing what this helps us do. The DMA address I can understand, it may be given to a remote core so we may want to verify it is the same on both sides, but the actual virtual kernel address is of no value to us, a hash to track it across uses is just as good. Andrew > regards > Suman > >> >> Andrew >> >> >>> regards >>> Suman >>> >>>> >>>> Andrew >>>> >>>> >>>>> Signed-off-by: Suman Anna <s-anna@ti.com> >>>>> --- >>>>> drivers/rpmsg/virtio_rpmsg_bus.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c >>>>> index f29dee731026..1345f373a1a0 100644 >>>>> --- a/drivers/rpmsg/virtio_rpmsg_bus.c >>>>> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c >>>>> @@ -950,7 +950,7 @@ static int rpmsg_probe(struct virtio_device *vdev) >>>>> goto vqs_del; >>>>> } >>>>> >>>>> - dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n", >>>>> + dev_dbg(&vdev->dev, "buffers: va %pK, dma %pad\n", >>>>> bufs_va, &vrp->bufs_dma); >>>>> >>>>> /* half of the buffers is dedicated for RX */ >>>>> >>> >
On Fri 09 Aug 13:25 PDT 2019, Suman Anna wrote: > Hi Bjorn, > Hi Suman > On 10/23/18 8:19 PM, Suman Anna wrote: > > The virtio_rpmsg_bus driver uses the "%p" format-specifier for > > printing the vring buffer address. This prints only a hashed > > pointer even for previliged users. Use "%pK" instead so that > > the address can be printed during debug using kptr_restrict > > sysctl. > > Seems to have been lost among the patches, can you pick up this trivial > patch for 5.4? Should apply cleanly on the latest HEAD as well. > I share Andrew's question regarding what benefit you have from knowing this value. Should we not just remove the va from the print? Or do you actually have a use case for it? Regards, Bjorn > regards > Suman > > > > > Signed-off-by: Suman Anna <s-anna@ti.com> > > --- > > drivers/rpmsg/virtio_rpmsg_bus.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c > > index f29dee731026..1345f373a1a0 100644 > > --- a/drivers/rpmsg/virtio_rpmsg_bus.c > > +++ b/drivers/rpmsg/virtio_rpmsg_bus.c > > @@ -950,7 +950,7 @@ static int rpmsg_probe(struct virtio_device *vdev) > > goto vqs_del; > > } > > > > - dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n", > > + dev_dbg(&vdev->dev, "buffers: va %pK, dma %pad\n", > > bufs_va, &vrp->bufs_dma); > > > > /* half of the buffers is dedicated for RX */ > > >
Hi Bjorn, On 8/27/19 12:10 AM, Bjorn Andersson wrote: > On Fri 09 Aug 13:25 PDT 2019, Suman Anna wrote: > >> Hi Bjorn, >> > > Hi Suman > >> On 10/23/18 8:19 PM, Suman Anna wrote: >>> The virtio_rpmsg_bus driver uses the "%p" format-specifier for >>> printing the vring buffer address. This prints only a hashed >>> pointer even for previliged users. Use "%pK" instead so that >>> the address can be printed during debug using kptr_restrict >>> sysctl. >> >> Seems to have been lost among the patches, can you pick up this trivial >> patch for 5.4? Should apply cleanly on the latest HEAD as well. >> > > I share Andrew's question regarding what benefit you have from knowing > this value. Should we not just remove the va from the print? Or do you > actually have a use case for it?. I mainly use it during debug when comparing against kernel_page_tables and vmallocinfo. The pools that we use are not always guaranteed to be from linear memory, and behavior changes when using with CMA or DMA pools. Note that usage of %pK does not leak the addresses automatically, but atleast enables me to get the values when needed. The changes also bring the usage in rpmsg core in sync with the remoteproc core. regards Suman > > Regards, > Bjorn > >> regards >> Suman >> >>> >>> Signed-off-by: Suman Anna <s-anna@ti.com> >>> --- >>> drivers/rpmsg/virtio_rpmsg_bus.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c >>> index f29dee731026..1345f373a1a0 100644 >>> --- a/drivers/rpmsg/virtio_rpmsg_bus.c >>> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c >>> @@ -950,7 +950,7 @@ static int rpmsg_probe(struct virtio_device *vdev) >>> goto vqs_del; >>> } >>> >>> - dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n", >>> + dev_dbg(&vdev->dev, "buffers: va %pK, dma %pad\n", >>> bufs_va, &vrp->bufs_dma); >>> >>> /* half of the buffers is dedicated for RX */ >>> >>
On Tue 27 Aug 13:25 PDT 2019, Suman Anna wrote: > Hi Bjorn, > > On 8/27/19 12:10 AM, Bjorn Andersson wrote: > > On Fri 09 Aug 13:25 PDT 2019, Suman Anna wrote: > > > >> Hi Bjorn, > >> > > > > Hi Suman > > > >> On 10/23/18 8:19 PM, Suman Anna wrote: > >>> The virtio_rpmsg_bus driver uses the "%p" format-specifier for > >>> printing the vring buffer address. This prints only a hashed > >>> pointer even for previliged users. Use "%pK" instead so that > >>> the address can be printed during debug using kptr_restrict > >>> sysctl. > >> > >> Seems to have been lost among the patches, can you pick up this trivial > >> patch for 5.4? Should apply cleanly on the latest HEAD as well. > >> > > > > I share Andrew's question regarding what benefit you have from knowing > > this value. Should we not just remove the va from the print? Or do you > > actually have a use case for it?. > > I mainly use it during debug when comparing against kernel_page_tables > and vmallocinfo. The pools that we use are not always guaranteed to be > from linear memory, and behavior changes when using with CMA or DMA pools. > Thanks Suman. It seems to me that there's room for improvement to aid this kind of debugging. But your usecase seems reasonable, so I'm merging the patch. > Note that usage of %pK does not leak the addresses automatically, but > atleast enables me to get the values when needed. The changes also bring > the usage in rpmsg core in sync with the remoteproc core. > Sounds like shouldn't have merged them in remoteproc then ;P Thanks, Bjorn > regards > Suman > > > > > Regards, > > Bjorn > > > >> regards > >> Suman > >> > >>> > >>> Signed-off-by: Suman Anna <s-anna@ti.com> > >>> --- > >>> drivers/rpmsg/virtio_rpmsg_bus.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c > >>> index f29dee731026..1345f373a1a0 100644 > >>> --- a/drivers/rpmsg/virtio_rpmsg_bus.c > >>> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c > >>> @@ -950,7 +950,7 @@ static int rpmsg_probe(struct virtio_device *vdev) > >>> goto vqs_del; > >>> } > >>> > >>> - dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n", > >>> + dev_dbg(&vdev->dev, "buffers: va %pK, dma %pad\n", > >>> bufs_va, &vrp->bufs_dma); > >>> > >>> /* half of the buffers is dedicated for RX */ > >>> > >> >
On 8/27/19 5:07 PM, Bjorn Andersson wrote: > On Tue 27 Aug 13:25 PDT 2019, Suman Anna wrote: > >> Hi Bjorn, >> >> On 8/27/19 12:10 AM, Bjorn Andersson wrote: >>> On Fri 09 Aug 13:25 PDT 2019, Suman Anna wrote: >>> >>>> Hi Bjorn, >>>> >>> >>> Hi Suman >>> >>>> On 10/23/18 8:19 PM, Suman Anna wrote: >>>>> The virtio_rpmsg_bus driver uses the "%p" format-specifier for >>>>> printing the vring buffer address. This prints only a hashed >>>>> pointer even for previliged users. Use "%pK" instead so that >>>>> the address can be printed during debug using kptr_restrict >>>>> sysctl. >>>> >>>> Seems to have been lost among the patches, can you pick up this trivial >>>> patch for 5.4? Should apply cleanly on the latest HEAD as well. >>>> >>> >>> I share Andrew's question regarding what benefit you have from knowing >>> this value. Should we not just remove the va from the print? Or do you >>> actually have a use case for it?. >> >> I mainly use it during debug when comparing against kernel_page_tables >> and vmallocinfo. The pools that we use are not always guaranteed to be >> from linear memory, and behavior changes when using with CMA or DMA pools. >> > > Thanks Suman. It seems to me that there's room for improvement to aid > this kind of debugging. But your usecase seems reasonable, so I'm > merging the patch. Thanks Bjorn. > >> Note that usage of %pK does not leak the addresses automatically, but >> atleast enables me to get the values when needed. The changes also bring >> the usage in rpmsg core in sync with the remoteproc core. >> > > Sounds like shouldn't have merged them in remoteproc then ;P Slightly different reasoning looking at the commit, it was probably when %p was leaking the addresses. regards Suman > > Thanks, > Bjorn > >> regards >> Suman >> >>> >>> Regards, >>> Bjorn >>> >>>> regards >>>> Suman >>>> >>>>> >>>>> Signed-off-by: Suman Anna <s-anna@ti.com> >>>>> --- >>>>> drivers/rpmsg/virtio_rpmsg_bus.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c >>>>> index f29dee731026..1345f373a1a0 100644 >>>>> --- a/drivers/rpmsg/virtio_rpmsg_bus.c >>>>> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c >>>>> @@ -950,7 +950,7 @@ static int rpmsg_probe(struct virtio_device *vdev) >>>>> goto vqs_del; >>>>> } >>>>> >>>>> - dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n", >>>>> + dev_dbg(&vdev->dev, "buffers: va %pK, dma %pad\n", >>>>> bufs_va, &vrp->bufs_dma); >>>>> >>>>> /* half of the buffers is dedicated for RX */ >>>>> >>>> >>
On 8/27/19 5:15 PM, Suman Anna wrote: > On 8/27/19 5:07 PM, Bjorn Andersson wrote: >> On Tue 27 Aug 13:25 PDT 2019, Suman Anna wrote: >> >>> Hi Bjorn, >>> >>> On 8/27/19 12:10 AM, Bjorn Andersson wrote: >>>> On Fri 09 Aug 13:25 PDT 2019, Suman Anna wrote: >>>> >>>>> Hi Bjorn, >>>>> >>>> >>>> Hi Suman >>>> >>>>> On 10/23/18 8:19 PM, Suman Anna wrote: >>>>>> The virtio_rpmsg_bus driver uses the "%p" format-specifier for >>>>>> printing the vring buffer address. This prints only a hashed >>>>>> pointer even for previliged users. Use "%pK" instead so that >>>>>> the address can be printed during debug using kptr_restrict >>>>>> sysctl. >>>>> >>>>> Seems to have been lost among the patches, can you pick up this trivial >>>>> patch for 5.4? Should apply cleanly on the latest HEAD as well. >>>>> >>>> >>>> I share Andrew's question regarding what benefit you have from knowing >>>> this value. Should we not just remove the va from the print? Or do you >>>> actually have a use case for it?. >>> >>> I mainly use it during debug when comparing against kernel_page_tables >>> and vmallocinfo. The pools that we use are not always guaranteed to be >>> from linear memory, and behavior changes when using with CMA or DMA pools. >>> >> >> Thanks Suman. It seems to me that there's room for improvement to aid >> this kind of debugging. But your usecase seems reasonable, so I'm >> merging the patch. > > Thanks Bjorn. Btw, looks like you applied the patch against rproc-next instead of rpmsg-next. regards Suman > >> >>> Note that usage of %pK does not leak the addresses automatically, but >>> atleast enables me to get the values when needed. The changes also bring >>> the usage in rpmsg core in sync with the remoteproc core. >>> >> >> Sounds like shouldn't have merged them in remoteproc then ;P > > Slightly different reasoning looking at the commit, it was probably when > %p was leaking the addresses. > > regards > Suman > >> >> Thanks, >> Bjorn >> >>> regards >>> Suman >>> >>>> >>>> Regards, >>>> Bjorn >>>> >>>>> regards >>>>> Suman >>>>> >>>>>> >>>>>> Signed-off-by: Suman Anna <s-anna@ti.com> >>>>>> --- >>>>>> drivers/rpmsg/virtio_rpmsg_bus.c | 2 +- >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c >>>>>> index f29dee731026..1345f373a1a0 100644 >>>>>> --- a/drivers/rpmsg/virtio_rpmsg_bus.c >>>>>> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c >>>>>> @@ -950,7 +950,7 @@ static int rpmsg_probe(struct virtio_device *vdev) >>>>>> goto vqs_del; >>>>>> } >>>>>> >>>>>> - dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n", >>>>>> + dev_dbg(&vdev->dev, "buffers: va %pK, dma %pad\n", >>>>>> bufs_va, &vrp->bufs_dma); >>>>>> >>>>>> /* half of the buffers is dedicated for RX */ >>>>>> >>>>> >>> >
On Tue 27 Aug 15:17 PDT 2019, Suman Anna wrote: > On 8/27/19 5:15 PM, Suman Anna wrote: > > On 8/27/19 5:07 PM, Bjorn Andersson wrote: > >> On Tue 27 Aug 13:25 PDT 2019, Suman Anna wrote: > >> > >>> Hi Bjorn, > >>> > >>> On 8/27/19 12:10 AM, Bjorn Andersson wrote: > >>>> On Fri 09 Aug 13:25 PDT 2019, Suman Anna wrote: > >>>> > >>>>> Hi Bjorn, > >>>>> > >>>> > >>>> Hi Suman > >>>> > >>>>> On 10/23/18 8:19 PM, Suman Anna wrote: > >>>>>> The virtio_rpmsg_bus driver uses the "%p" format-specifier for > >>>>>> printing the vring buffer address. This prints only a hashed > >>>>>> pointer even for previliged users. Use "%pK" instead so that > >>>>>> the address can be printed during debug using kptr_restrict > >>>>>> sysctl. > >>>>> > >>>>> Seems to have been lost among the patches, can you pick up this trivial > >>>>> patch for 5.4? Should apply cleanly on the latest HEAD as well. > >>>>> > >>>> > >>>> I share Andrew's question regarding what benefit you have from knowing > >>>> this value. Should we not just remove the va from the print? Or do you > >>>> actually have a use case for it?. > >>> > >>> I mainly use it during debug when comparing against kernel_page_tables > >>> and vmallocinfo. The pools that we use are not always guaranteed to be > >>> from linear memory, and behavior changes when using with CMA or DMA pools. > >>> > >> > >> Thanks Suman. It seems to me that there's room for improvement to aid > >> this kind of debugging. But your usecase seems reasonable, so I'm > >> merging the patch. > > > > Thanks Bjorn. > > Btw, looks like you applied the patch against rproc-next instead of > rpmsg-next. > Thanks for noticing so quick; I moved the change to the correct branch. Regards, Bjorn > regards > Suman > > > > >> > >>> Note that usage of %pK does not leak the addresses automatically, but > >>> atleast enables me to get the values when needed. The changes also bring > >>> the usage in rpmsg core in sync with the remoteproc core. > >>> > >> > >> Sounds like shouldn't have merged them in remoteproc then ;P > > > > Slightly different reasoning looking at the commit, it was probably when > > %p was leaking the addresses. > > > > regards > > Suman > > > >> > >> Thanks, > >> Bjorn > >> > >>> regards > >>> Suman > >>> > >>>> > >>>> Regards, > >>>> Bjorn > >>>> > >>>>> regards > >>>>> Suman > >>>>> > >>>>>> > >>>>>> Signed-off-by: Suman Anna <s-anna@ti.com> > >>>>>> --- > >>>>>> drivers/rpmsg/virtio_rpmsg_bus.c | 2 +- > >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>>>> > >>>>>> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c > >>>>>> index f29dee731026..1345f373a1a0 100644 > >>>>>> --- a/drivers/rpmsg/virtio_rpmsg_bus.c > >>>>>> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c > >>>>>> @@ -950,7 +950,7 @@ static int rpmsg_probe(struct virtio_device *vdev) > >>>>>> goto vqs_del; > >>>>>> } > >>>>>> > >>>>>> - dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n", > >>>>>> + dev_dbg(&vdev->dev, "buffers: va %pK, dma %pad\n", > >>>>>> bufs_va, &vrp->bufs_dma); > >>>>>> > >>>>>> /* half of the buffers is dedicated for RX */ > >>>>>> > >>>>> > >>> > > >
diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c index f29dee731026..1345f373a1a0 100644 --- a/drivers/rpmsg/virtio_rpmsg_bus.c +++ b/drivers/rpmsg/virtio_rpmsg_bus.c @@ -950,7 +950,7 @@ static int rpmsg_probe(struct virtio_device *vdev) goto vqs_del; } - dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n", + dev_dbg(&vdev->dev, "buffers: va %pK, dma %pad\n", bufs_va, &vrp->bufs_dma); /* half of the buffers is dedicated for RX */
The virtio_rpmsg_bus driver uses the "%p" format-specifier for printing the vring buffer address. This prints only a hashed pointer even for previliged users. Use "%pK" instead so that the address can be printed during debug using kptr_restrict sysctl. Signed-off-by: Suman Anna <s-anna@ti.com> --- drivers/rpmsg/virtio_rpmsg_bus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)