Re: drm/amd/display: Add HDCP module - static analysis bug report
On Tue, Nov 5, 2019 at 3:39 PM Harry Wentland wrote: > > > > On 2019-11-05 9:23 a.m., Daniel Vetter wrote: > > On Tue, Nov 5, 2019 at 3:17 PM Harry Wentland wrote: > >> > >> > >> > >> On 2019-11-05 8:14 a.m., Daniel Vetter wrote: > >>> On Tue, Nov 5, 2019 at 1:52 PM Alex Deucher wrote: > > On Mon, Nov 4, 2019 at 12:24 PM Daniel Vetter wrote: > > > > On Mon, Nov 04, 2019 at 12:05:40PM -0500, Alex Deucher wrote: > >> On Mon, Nov 4, 2019 at 11:55 AM Daniel Vetter wrote: > >>> > >>> On Mon, Nov 04, 2019 at 03:23:09PM +, Harry Wentland wrote: > On 2019-11-04 5:53 a.m., Daniel Vetter wrote: > > On Wed, Oct 9, 2019 at 10:58 PM Daniel Vetter > > wrote: > >> On Wed, Oct 9, 2019 at 10:46 PM Lakha, Bhawanpreet > >> wrote: > >>> > >>> I misunderstood and was talking about the ksv validation > >>> specifically > >>> (usage of drm_hdcp_check_ksvs_revoked()). > >> > >> Hm for that specifically I think you want to do both, i.e. both > >> consult your psp, but also check for revoked ksvs with the core > >> helper. At least on some platforms only the core helper might have > >> the > >> updated revoke list. > >> > > I think it's an either/or. Either we use an HDCP implementation > that's > fully running in x86 kernel space (still not sure how that's > compliant) > or we fully rely on our PSP FW to do what it's designed to do. I > don't > think it makes sense to mix and match here. > >>> > >>> Then you need to somehow tie the revoke list that's in the psp to the > >>> revoke list update logic we have. That's what we've done for hdcp2 > >>> (which > >>> is similarly to yours implemented in hw). The point is that on linux > >>> we > >>> now have a standard way to get these revoke lists updated/handled. > >>> > >>> I guess it wasn't clear how exactly I think you're supposed to combine > >>> them? > >> > >> There's no driver sw required at all for our implementation and as far > >> as I know, HDCP 2.x requires that all of the key revoke handling be > >> handled in a secure processor rather than than on the host processor, > >> so I'm not sure how we make use if it. All the driver sw is > >> responsible for doing is saving/restoring the potentially updated srm > >> at suspend/resume/etc. > > > > Uh, you don't have a permanent store on the chip? I thought another > > requirement is that you can't downgrade. > > Right. That's why the driver has to save and restore the srm when the > GPU is powered down. I guess that part can be done by the host > processor as long as the srm is signed properly. > > > > > And for hw solutions all you do with the updated revoke cert is stuff it > > into the hw, it's purely for updating it. And those updates need to come > > from somewhere else (usually in the media you play), the kernel can't > > fetch them over the internet itself. I thought we already had the > > function > > to give you the srm directly so you can stuff it into the hw, but looks > > like that part isn't there (yet). > > IIRC, the revoke stuff gets gleaned from the stream by the secure > processor somehow when you play back secure content. I'm not entirely > clear on the details, but from the design, the driver doesn't have to > do anything in our case other than saving and restoring the srm from > the secure processor. > >>> > >>> Hm, is that implemented in open userspace somewhere? tbh I don't know > >>> whether the srm is in the bitstream or somewhere else in the file > >>> (they're all containers with lots of stuff), but the current upstream > >>> hdcp stuff is done under the assumption that userspace still does the > >>> decrypting (so only the lowest content protection level supported > >>> right now). Hence the explicit step to update the kernel on the latest > >>> srm, which the kernel can then use to either check for revokes or hand > >>> to the hardware. > >>> -Daniel > >>> > >> > >> Not sure I follow your questions about whether this is implemented in > >> open userspace. > >> > >> The SRM is provided to PSP (our secure processor) through other > >> interfaces. I'm currently not sure whether that's directly from the > >> bitstream or another interface from the secure userspace that's handling > >> content protection (e.g. OEMCrypto or similar). > > > > Well if the full thing needs the blob (otherwise how do you get at the > > SRM), then the blob needs to be open, or we need something else. > > > >> The key for HDCP SRM handling is that PSP doesn't have a permanent store > >> on the chip and needs us to handle the save and restore at > >> boot/shutdown/suspend/resume. Think of it as an initialization and >
Re: drm/amd/display: Add HDCP module - static analysis bug report
On 2019-11-05 9:23 a.m., Daniel Vetter wrote: > On Tue, Nov 5, 2019 at 3:17 PM Harry Wentland wrote: >> >> >> >> On 2019-11-05 8:14 a.m., Daniel Vetter wrote: >>> On Tue, Nov 5, 2019 at 1:52 PM Alex Deucher wrote: On Mon, Nov 4, 2019 at 12:24 PM Daniel Vetter wrote: > > On Mon, Nov 04, 2019 at 12:05:40PM -0500, Alex Deucher wrote: >> On Mon, Nov 4, 2019 at 11:55 AM Daniel Vetter wrote: >>> >>> On Mon, Nov 04, 2019 at 03:23:09PM +, Harry Wentland wrote: On 2019-11-04 5:53 a.m., Daniel Vetter wrote: > On Wed, Oct 9, 2019 at 10:58 PM Daniel Vetter wrote: >> On Wed, Oct 9, 2019 at 10:46 PM Lakha, Bhawanpreet >> wrote: >>> >>> I misunderstood and was talking about the ksv validation >>> specifically >>> (usage of drm_hdcp_check_ksvs_revoked()). >> >> Hm for that specifically I think you want to do both, i.e. both >> consult your psp, but also check for revoked ksvs with the core >> helper. At least on some platforms only the core helper might have >> the >> updated revoke list. >> I think it's an either/or. Either we use an HDCP implementation that's fully running in x86 kernel space (still not sure how that's compliant) or we fully rely on our PSP FW to do what it's designed to do. I don't think it makes sense to mix and match here. >>> >>> Then you need to somehow tie the revoke list that's in the psp to the >>> revoke list update logic we have. That's what we've done for hdcp2 >>> (which >>> is similarly to yours implemented in hw). The point is that on linux we >>> now have a standard way to get these revoke lists updated/handled. >>> >>> I guess it wasn't clear how exactly I think you're supposed to combine >>> them? >> >> There's no driver sw required at all for our implementation and as far >> as I know, HDCP 2.x requires that all of the key revoke handling be >> handled in a secure processor rather than than on the host processor, >> so I'm not sure how we make use if it. All the driver sw is >> responsible for doing is saving/restoring the potentially updated srm >> at suspend/resume/etc. > > Uh, you don't have a permanent store on the chip? I thought another > requirement is that you can't downgrade. Right. That's why the driver has to save and restore the srm when the GPU is powered down. I guess that part can be done by the host processor as long as the srm is signed properly. > > And for hw solutions all you do with the updated revoke cert is stuff it > into the hw, it's purely for updating it. And those updates need to come > from somewhere else (usually in the media you play), the kernel can't > fetch them over the internet itself. I thought we already had the function > to give you the srm directly so you can stuff it into the hw, but looks > like that part isn't there (yet). IIRC, the revoke stuff gets gleaned from the stream by the secure processor somehow when you play back secure content. I'm not entirely clear on the details, but from the design, the driver doesn't have to do anything in our case other than saving and restoring the srm from the secure processor. >>> >>> Hm, is that implemented in open userspace somewhere? tbh I don't know >>> whether the srm is in the bitstream or somewhere else in the file >>> (they're all containers with lots of stuff), but the current upstream >>> hdcp stuff is done under the assumption that userspace still does the >>> decrypting (so only the lowest content protection level supported >>> right now). Hence the explicit step to update the kernel on the latest >>> srm, which the kernel can then use to either check for revokes or hand >>> to the hardware. >>> -Daniel >>> >> >> Not sure I follow your questions about whether this is implemented in >> open userspace. >> >> The SRM is provided to PSP (our secure processor) through other >> interfaces. I'm currently not sure whether that's directly from the >> bitstream or another interface from the secure userspace that's handling >> content protection (e.g. OEMCrypto or similar). > > Well if the full thing needs the blob (otherwise how do you get at the > SRM), then the blob needs to be open, or we need something else. > >> The key for HDCP SRM handling is that PSP doesn't have a permanent store >> on the chip and needs us to handle the save and restore at >> boot/shutdown/suspend/resume. Think of it as an initialization and >> teardown step of PSP. >> >> The idea is to provide an amdgpu device-specific sysfs that's used to >> save/restore the SRM without any handling in the kernel (unlike the work >> done by Ramalingam to do the revocation check in DRM). This sysfs will >> be called by a simple init script to store and read the SRM from
Re: drm/amd/display: Add HDCP module - static analysis bug report
On Tue, Nov 5, 2019 at 3:17 PM Harry Wentland wrote: > > > > On 2019-11-05 8:14 a.m., Daniel Vetter wrote: > > On Tue, Nov 5, 2019 at 1:52 PM Alex Deucher wrote: > >> > >> On Mon, Nov 4, 2019 at 12:24 PM Daniel Vetter wrote: > >>> > >>> On Mon, Nov 04, 2019 at 12:05:40PM -0500, Alex Deucher wrote: > On Mon, Nov 4, 2019 at 11:55 AM Daniel Vetter wrote: > > > > On Mon, Nov 04, 2019 at 03:23:09PM +, Harry Wentland wrote: > >> On 2019-11-04 5:53 a.m., Daniel Vetter wrote: > >>> On Wed, Oct 9, 2019 at 10:58 PM Daniel Vetter wrote: > On Wed, Oct 9, 2019 at 10:46 PM Lakha, Bhawanpreet > wrote: > > > > I misunderstood and was talking about the ksv validation > > specifically > > (usage of drm_hdcp_check_ksvs_revoked()). > > Hm for that specifically I think you want to do both, i.e. both > consult your psp, but also check for revoked ksvs with the core > helper. At least on some platforms only the core helper might have > the > updated revoke list. > > >> > >> I think it's an either/or. Either we use an HDCP implementation that's > >> fully running in x86 kernel space (still not sure how that's compliant) > >> or we fully rely on our PSP FW to do what it's designed to do. I don't > >> think it makes sense to mix and match here. > > > > Then you need to somehow tie the revoke list that's in the psp to the > > revoke list update logic we have. That's what we've done for hdcp2 > > (which > > is similarly to yours implemented in hw). The point is that on linux we > > now have a standard way to get these revoke lists updated/handled. > > > > I guess it wasn't clear how exactly I think you're supposed to combine > > them? > > There's no driver sw required at all for our implementation and as far > as I know, HDCP 2.x requires that all of the key revoke handling be > handled in a secure processor rather than than on the host processor, > so I'm not sure how we make use if it. All the driver sw is > responsible for doing is saving/restoring the potentially updated srm > at suspend/resume/etc. > >>> > >>> Uh, you don't have a permanent store on the chip? I thought another > >>> requirement is that you can't downgrade. > >> > >> Right. That's why the driver has to save and restore the srm when the > >> GPU is powered down. I guess that part can be done by the host > >> processor as long as the srm is signed properly. > >> > >>> > >>> And for hw solutions all you do with the updated revoke cert is stuff it > >>> into the hw, it's purely for updating it. And those updates need to come > >>> from somewhere else (usually in the media you play), the kernel can't > >>> fetch them over the internet itself. I thought we already had the function > >>> to give you the srm directly so you can stuff it into the hw, but looks > >>> like that part isn't there (yet). > >> > >> IIRC, the revoke stuff gets gleaned from the stream by the secure > >> processor somehow when you play back secure content. I'm not entirely > >> clear on the details, but from the design, the driver doesn't have to > >> do anything in our case other than saving and restoring the srm from > >> the secure processor. > > > > Hm, is that implemented in open userspace somewhere? tbh I don't know > > whether the srm is in the bitstream or somewhere else in the file > > (they're all containers with lots of stuff), but the current upstream > > hdcp stuff is done under the assumption that userspace still does the > > decrypting (so only the lowest content protection level supported > > right now). Hence the explicit step to update the kernel on the latest > > srm, which the kernel can then use to either check for revokes or hand > > to the hardware. > > -Daniel > > > > Not sure I follow your questions about whether this is implemented in > open userspace. > > The SRM is provided to PSP (our secure processor) through other > interfaces. I'm currently not sure whether that's directly from the > bitstream or another interface from the secure userspace that's handling > content protection (e.g. OEMCrypto or similar). Well if the full thing needs the blob (otherwise how do you get at the SRM), then the blob needs to be open, or we need something else. > The key for HDCP SRM handling is that PSP doesn't have a permanent store > on the chip and needs us to handle the save and restore at > boot/shutdown/suspend/resume. Think of it as an initialization and > teardown step of PSP. > > The idea is to provide an amdgpu device-specific sysfs that's used to > save/restore the SRM without any handling in the kernel (unlike the work > done by Ramalingam to do the revocation check in DRM). This sysfs will > be called by a simple init script to store and read the SRM from disk. Uh that's what I meant, now we'll end up with 2 ways to handle the SRM. We
Re: drm/amd/display: Add HDCP module - static analysis bug report
On 2019-11-05 8:14 a.m., Daniel Vetter wrote: > On Tue, Nov 5, 2019 at 1:52 PM Alex Deucher wrote: >> >> On Mon, Nov 4, 2019 at 12:24 PM Daniel Vetter wrote: >>> >>> On Mon, Nov 04, 2019 at 12:05:40PM -0500, Alex Deucher wrote: On Mon, Nov 4, 2019 at 11:55 AM Daniel Vetter wrote: > > On Mon, Nov 04, 2019 at 03:23:09PM +, Harry Wentland wrote: >> On 2019-11-04 5:53 a.m., Daniel Vetter wrote: >>> On Wed, Oct 9, 2019 at 10:58 PM Daniel Vetter wrote: On Wed, Oct 9, 2019 at 10:46 PM Lakha, Bhawanpreet wrote: > > I misunderstood and was talking about the ksv validation specifically > (usage of drm_hdcp_check_ksvs_revoked()). Hm for that specifically I think you want to do both, i.e. both consult your psp, but also check for revoked ksvs with the core helper. At least on some platforms only the core helper might have the updated revoke list. >> >> I think it's an either/or. Either we use an HDCP implementation that's >> fully running in x86 kernel space (still not sure how that's compliant) >> or we fully rely on our PSP FW to do what it's designed to do. I don't >> think it makes sense to mix and match here. > > Then you need to somehow tie the revoke list that's in the psp to the > revoke list update logic we have. That's what we've done for hdcp2 (which > is similarly to yours implemented in hw). The point is that on linux we > now have a standard way to get these revoke lists updated/handled. > > I guess it wasn't clear how exactly I think you're supposed to combine > them? There's no driver sw required at all for our implementation and as far as I know, HDCP 2.x requires that all of the key revoke handling be handled in a secure processor rather than than on the host processor, so I'm not sure how we make use if it. All the driver sw is responsible for doing is saving/restoring the potentially updated srm at suspend/resume/etc. >>> >>> Uh, you don't have a permanent store on the chip? I thought another >>> requirement is that you can't downgrade. >> >> Right. That's why the driver has to save and restore the srm when the >> GPU is powered down. I guess that part can be done by the host >> processor as long as the srm is signed properly. >> >>> >>> And for hw solutions all you do with the updated revoke cert is stuff it >>> into the hw, it's purely for updating it. And those updates need to come >>> from somewhere else (usually in the media you play), the kernel can't >>> fetch them over the internet itself. I thought we already had the function >>> to give you the srm directly so you can stuff it into the hw, but looks >>> like that part isn't there (yet). >> >> IIRC, the revoke stuff gets gleaned from the stream by the secure >> processor somehow when you play back secure content. I'm not entirely >> clear on the details, but from the design, the driver doesn't have to >> do anything in our case other than saving and restoring the srm from >> the secure processor. > > Hm, is that implemented in open userspace somewhere? tbh I don't know > whether the srm is in the bitstream or somewhere else in the file > (they're all containers with lots of stuff), but the current upstream > hdcp stuff is done under the assumption that userspace still does the > decrypting (so only the lowest content protection level supported > right now). Hence the explicit step to update the kernel on the latest > srm, which the kernel can then use to either check for revokes or hand > to the hardware. > -Daniel > Not sure I follow your questions about whether this is implemented in open userspace. The SRM is provided to PSP (our secure processor) through other interfaces. I'm currently not sure whether that's directly from the bitstream or another interface from the secure userspace that's handling content protection (e.g. OEMCrypto or similar). The key for HDCP SRM handling is that PSP doesn't have a permanent store on the chip and needs us to handle the save and restore at boot/shutdown/suspend/resume. Think of it as an initialization and teardown step of PSP. The idea is to provide an amdgpu device-specific sysfs that's used to save/restore the SRM without any handling in the kernel (unlike the work done by Ramalingam to do the revocation check in DRM). This sysfs will be called by a simple init script to store and read the SRM from disk. Harry >> Alex >> >>> -Daniel >>> Alex > -Daniel > > >> > For the defines I will create patches to use drm_hdcp where it is > usable. Thanks a lot. Ime once we have shared definitions it's much easier to also share some helpers, where it makes sense. Aside I think the hdcp code could also use a bit of demidlayering. At least I'm not understanding why you add a 2nd abstraction
Re: drm/amd/display: Add HDCP module - static analysis bug report
On Tue, Nov 5, 2019 at 1:52 PM Alex Deucher wrote: > > On Mon, Nov 4, 2019 at 12:24 PM Daniel Vetter wrote: > > > > On Mon, Nov 04, 2019 at 12:05:40PM -0500, Alex Deucher wrote: > > > On Mon, Nov 4, 2019 at 11:55 AM Daniel Vetter wrote: > > > > > > > > On Mon, Nov 04, 2019 at 03:23:09PM +, Harry Wentland wrote: > > > > > On 2019-11-04 5:53 a.m., Daniel Vetter wrote: > > > > > > On Wed, Oct 9, 2019 at 10:58 PM Daniel Vetter > > > > > > wrote: > > > > > >> On Wed, Oct 9, 2019 at 10:46 PM Lakha, Bhawanpreet > > > > > >> wrote: > > > > > >>> > > > > > >>> I misunderstood and was talking about the ksv validation > > > > > >>> specifically > > > > > >>> (usage of drm_hdcp_check_ksvs_revoked()). > > > > > >> > > > > > >> Hm for that specifically I think you want to do both, i.e. both > > > > > >> consult your psp, but also check for revoked ksvs with the core > > > > > >> helper. At least on some platforms only the core helper might have > > > > > >> the > > > > > >> updated revoke list. > > > > > >> > > > > > > > > > > I think it's an either/or. Either we use an HDCP implementation that's > > > > > fully running in x86 kernel space (still not sure how that's > > > > > compliant) > > > > > or we fully rely on our PSP FW to do what it's designed to do. I don't > > > > > think it makes sense to mix and match here. > > > > > > > > Then you need to somehow tie the revoke list that's in the psp to the > > > > revoke list update logic we have. That's what we've done for hdcp2 > > > > (which > > > > is similarly to yours implemented in hw). The point is that on linux we > > > > now have a standard way to get these revoke lists updated/handled. > > > > > > > > I guess it wasn't clear how exactly I think you're supposed to combine > > > > them? > > > > > > There's no driver sw required at all for our implementation and as far > > > as I know, HDCP 2.x requires that all of the key revoke handling be > > > handled in a secure processor rather than than on the host processor, > > > so I'm not sure how we make use if it. All the driver sw is > > > responsible for doing is saving/restoring the potentially updated srm > > > at suspend/resume/etc. > > > > Uh, you don't have a permanent store on the chip? I thought another > > requirement is that you can't downgrade. > > Right. That's why the driver has to save and restore the srm when the > GPU is powered down. I guess that part can be done by the host > processor as long as the srm is signed properly. > > > > > And for hw solutions all you do with the updated revoke cert is stuff it > > into the hw, it's purely for updating it. And those updates need to come > > from somewhere else (usually in the media you play), the kernel can't > > fetch them over the internet itself. I thought we already had the function > > to give you the srm directly so you can stuff it into the hw, but looks > > like that part isn't there (yet). > > IIRC, the revoke stuff gets gleaned from the stream by the secure > processor somehow when you play back secure content. I'm not entirely > clear on the details, but from the design, the driver doesn't have to > do anything in our case other than saving and restoring the srm from > the secure processor. Hm, is that implemented in open userspace somewhere? tbh I don't know whether the srm is in the bitstream or somewhere else in the file (they're all containers with lots of stuff), but the current upstream hdcp stuff is done under the assumption that userspace still does the decrypting (so only the lowest content protection level supported right now). Hence the explicit step to update the kernel on the latest srm, which the kernel can then use to either check for revokes or hand to the hardware. -Daniel > Alex > > > -Daniel > > > > > > > > Alex > > > > > > > -Daniel > > > > > > > > > > > > > > > > > > >>> For the defines I will create patches to use drm_hdcp where it is > > > > > >>> usable. > > > > > >> > > > > > >> Thanks a lot. Ime once we have shared definitions it's much easier > > > > > >> to > > > > > >> also share some helpers, where it makes sense. > > > > > >> > > > > > >> Aside I think the hdcp code could also use a bit of demidlayering. > > > > > >> At > > > > > >> least I'm not understanding why you add a 2nd abstraction layer for > > > > > >> i2c/dpcd, dm_helper already has that. That seems like one > > > > > >> abstraction > > > > > >> layer too much. > > > > > > > > > > > > I haven't seen anything fly by or in the latest pull request ... you > > > > > > folks still working on this or more put on the "maybe, probably > > > > > > never" > > > > > > pile? > > > > > > > > > > > > > > > > Following up with Bhawan. > > > > > > > > > > Harry > > > > > > > > > > > -Daniel > > > > > > > > > > > > > > > > > >> -Daniel > > > > > >> > > > > > >>> > > > > > >>> > > > > > >>> Bhawan > > > > > >>> > > > > > >>> On 2019-10-09 2:43 p.m., Daniel Vetter wrote: > > > > > On Wed, Oct 9, 2019 at 8:23 PM Lakha, Bhawanpreet > > > > >
Re: drm/amd/display: Add HDCP module - static analysis bug report
On Mon, Nov 4, 2019 at 12:24 PM Daniel Vetter wrote: > > On Mon, Nov 04, 2019 at 12:05:40PM -0500, Alex Deucher wrote: > > On Mon, Nov 4, 2019 at 11:55 AM Daniel Vetter wrote: > > > > > > On Mon, Nov 04, 2019 at 03:23:09PM +, Harry Wentland wrote: > > > > On 2019-11-04 5:53 a.m., Daniel Vetter wrote: > > > > > On Wed, Oct 9, 2019 at 10:58 PM Daniel Vetter wrote: > > > > >> On Wed, Oct 9, 2019 at 10:46 PM Lakha, Bhawanpreet > > > > >> wrote: > > > > >>> > > > > >>> I misunderstood and was talking about the ksv validation > > > > >>> specifically > > > > >>> (usage of drm_hdcp_check_ksvs_revoked()). > > > > >> > > > > >> Hm for that specifically I think you want to do both, i.e. both > > > > >> consult your psp, but also check for revoked ksvs with the core > > > > >> helper. At least on some platforms only the core helper might have > > > > >> the > > > > >> updated revoke list. > > > > >> > > > > > > > > I think it's an either/or. Either we use an HDCP implementation that's > > > > fully running in x86 kernel space (still not sure how that's compliant) > > > > or we fully rely on our PSP FW to do what it's designed to do. I don't > > > > think it makes sense to mix and match here. > > > > > > Then you need to somehow tie the revoke list that's in the psp to the > > > revoke list update logic we have. That's what we've done for hdcp2 (which > > > is similarly to yours implemented in hw). The point is that on linux we > > > now have a standard way to get these revoke lists updated/handled. > > > > > > I guess it wasn't clear how exactly I think you're supposed to combine > > > them? > > > > There's no driver sw required at all for our implementation and as far > > as I know, HDCP 2.x requires that all of the key revoke handling be > > handled in a secure processor rather than than on the host processor, > > so I'm not sure how we make use if it. All the driver sw is > > responsible for doing is saving/restoring the potentially updated srm > > at suspend/resume/etc. > > Uh, you don't have a permanent store on the chip? I thought another > requirement is that you can't downgrade. Right. That's why the driver has to save and restore the srm when the GPU is powered down. I guess that part can be done by the host processor as long as the srm is signed properly. > > And for hw solutions all you do with the updated revoke cert is stuff it > into the hw, it's purely for updating it. And those updates need to come > from somewhere else (usually in the media you play), the kernel can't > fetch them over the internet itself. I thought we already had the function > to give you the srm directly so you can stuff it into the hw, but looks > like that part isn't there (yet). IIRC, the revoke stuff gets gleaned from the stream by the secure processor somehow when you play back secure content. I'm not entirely clear on the details, but from the design, the driver doesn't have to do anything in our case other than saving and restoring the srm from the secure processor. Alex > -Daniel > > > > > Alex > > > > > -Daniel > > > > > > > > > > > > > > >>> For the defines I will create patches to use drm_hdcp where it is > > > > >>> usable. > > > > >> > > > > >> Thanks a lot. Ime once we have shared definitions it's much easier to > > > > >> also share some helpers, where it makes sense. > > > > >> > > > > >> Aside I think the hdcp code could also use a bit of demidlayering. At > > > > >> least I'm not understanding why you add a 2nd abstraction layer for > > > > >> i2c/dpcd, dm_helper already has that. That seems like one abstraction > > > > >> layer too much. > > > > > > > > > > I haven't seen anything fly by or in the latest pull request ... you > > > > > folks still working on this or more put on the "maybe, probably never" > > > > > pile? > > > > > > > > > > > > > Following up with Bhawan. > > > > > > > > Harry > > > > > > > > > -Daniel > > > > > > > > > > > > > > >> -Daniel > > > > >> > > > > >>> > > > > >>> > > > > >>> Bhawan > > > > >>> > > > > >>> On 2019-10-09 2:43 p.m., Daniel Vetter wrote: > > > > On Wed, Oct 9, 2019 at 8:23 PM Lakha, Bhawanpreet > > > > wrote: > > > > > Hi, > > > > > > > > > > The reason we don't use drm_hdcp is because our policy is to do > > > > > hdcp > > > > > verification using PSP/HW (onboard secure processor). > > > > i915 also uses hw to auth, we still use the parts from drm_hdcp ... > > > > Did you actually look at what's in there? It's essentially just > > > > shared > > > > defines and data structures from the standard, plus a few minimal > > > > helpers to en/decode some bits. Just from a quick read the entire > > > > patch very much looks like midlayer everywhere design that we > > > > discussed back when DC landed ... > > > > -Daniel > > > > > > > > > Bhawan > > > > > > > > > > On 2019-10-09 12:32 p.m., Daniel Vetter wrote: > > > > >> On Thu, Oct 03, 2019 at 11:08:03PM +0100,
Re: drm/amd/display: Add HDCP module - static analysis bug report
On Mon, Nov 04, 2019 at 12:05:40PM -0500, Alex Deucher wrote: > On Mon, Nov 4, 2019 at 11:55 AM Daniel Vetter wrote: > > > > On Mon, Nov 04, 2019 at 03:23:09PM +, Harry Wentland wrote: > > > On 2019-11-04 5:53 a.m., Daniel Vetter wrote: > > > > On Wed, Oct 9, 2019 at 10:58 PM Daniel Vetter wrote: > > > >> On Wed, Oct 9, 2019 at 10:46 PM Lakha, Bhawanpreet > > > >> wrote: > > > >>> > > > >>> I misunderstood and was talking about the ksv validation specifically > > > >>> (usage of drm_hdcp_check_ksvs_revoked()). > > > >> > > > >> Hm for that specifically I think you want to do both, i.e. both > > > >> consult your psp, but also check for revoked ksvs with the core > > > >> helper. At least on some platforms only the core helper might have the > > > >> updated revoke list. > > > >> > > > > > > I think it's an either/or. Either we use an HDCP implementation that's > > > fully running in x86 kernel space (still not sure how that's compliant) > > > or we fully rely on our PSP FW to do what it's designed to do. I don't > > > think it makes sense to mix and match here. > > > > Then you need to somehow tie the revoke list that's in the psp to the > > revoke list update logic we have. That's what we've done for hdcp2 (which > > is similarly to yours implemented in hw). The point is that on linux we > > now have a standard way to get these revoke lists updated/handled. > > > > I guess it wasn't clear how exactly I think you're supposed to combine > > them? > > There's no driver sw required at all for our implementation and as far > as I know, HDCP 2.x requires that all of the key revoke handling be > handled in a secure processor rather than than on the host processor, > so I'm not sure how we make use if it. All the driver sw is > responsible for doing is saving/restoring the potentially updated srm > at suspend/resume/etc. Uh, you don't have a permanent store on the chip? I thought another requirement is that you can't downgrade. And for hw solutions all you do with the updated revoke cert is stuff it into the hw, it's purely for updating it. And those updates need to come from somewhere else (usually in the media you play), the kernel can't fetch them over the internet itself. I thought we already had the function to give you the srm directly so you can stuff it into the hw, but looks like that part isn't there (yet). -Daniel > > Alex > > > -Daniel > > > > > > > > > > >>> For the defines I will create patches to use drm_hdcp where it is > > > >>> usable. > > > >> > > > >> Thanks a lot. Ime once we have shared definitions it's much easier to > > > >> also share some helpers, where it makes sense. > > > >> > > > >> Aside I think the hdcp code could also use a bit of demidlayering. At > > > >> least I'm not understanding why you add a 2nd abstraction layer for > > > >> i2c/dpcd, dm_helper already has that. That seems like one abstraction > > > >> layer too much. > > > > > > > > I haven't seen anything fly by or in the latest pull request ... you > > > > folks still working on this or more put on the "maybe, probably never" > > > > pile? > > > > > > > > > > Following up with Bhawan. > > > > > > Harry > > > > > > > -Daniel > > > > > > > > > > > >> -Daniel > > > >> > > > >>> > > > >>> > > > >>> Bhawan > > > >>> > > > >>> On 2019-10-09 2:43 p.m., Daniel Vetter wrote: > > > On Wed, Oct 9, 2019 at 8:23 PM Lakha, Bhawanpreet > > > wrote: > > > > Hi, > > > > > > > > The reason we don't use drm_hdcp is because our policy is to do hdcp > > > > verification using PSP/HW (onboard secure processor). > > > i915 also uses hw to auth, we still use the parts from drm_hdcp ... > > > Did you actually look at what's in there? It's essentially just > > > shared > > > defines and data structures from the standard, plus a few minimal > > > helpers to en/decode some bits. Just from a quick read the entire > > > patch very much looks like midlayer everywhere design that we > > > discussed back when DC landed ... > > > -Daniel > > > > > > > Bhawan > > > > > > > > On 2019-10-09 12:32 p.m., Daniel Vetter wrote: > > > >> On Thu, Oct 03, 2019 at 11:08:03PM +0100, Colin Ian King wrote: > > > >>> Hi, > > > >>> > > > >>> Static analysis with Coverity has detected a potential issue with > > > >>> function validate_bksv in > > > >>> drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c with > > > >>> recent > > > >>> commit: > > > >>> > > > >>> commit ed9d8e2bcb003ec94658cafe9b1bb3960e2139ec > > > >>> Author: Bhawanpreet Lakha > > > >>> Date: Tue Aug 6 17:52:01 2019 -0400 > > > >>> > > > >>> drm/amd/display: Add HDCP module > > > >> I think the real question here is ... why is this not using > > > >> drm_hdcp? > > > >> -Daniel > > > >> > > > >>> The analysis is as follows: > > > >>> > > > >>>28 static inline enum mod_hdcp_status validate_bksv(struct
Re: drm/amd/display: Add HDCP module - static analysis bug report
On Mon, Nov 4, 2019 at 11:55 AM Daniel Vetter wrote:
>
> On Mon, Nov 04, 2019 at 03:23:09PM +, Harry Wentland wrote:
> > On 2019-11-04 5:53 a.m., Daniel Vetter wrote:
> > > On Wed, Oct 9, 2019 at 10:58 PM Daniel Vetter wrote:
> > >> On Wed, Oct 9, 2019 at 10:46 PM Lakha, Bhawanpreet
> > >> wrote:
> > >>>
> > >>> I misunderstood and was talking about the ksv validation specifically
> > >>> (usage of drm_hdcp_check_ksvs_revoked()).
> > >>
> > >> Hm for that specifically I think you want to do both, i.e. both
> > >> consult your psp, but also check for revoked ksvs with the core
> > >> helper. At least on some platforms only the core helper might have the
> > >> updated revoke list.
> > >>
> >
> > I think it's an either/or. Either we use an HDCP implementation that's
> > fully running in x86 kernel space (still not sure how that's compliant)
> > or we fully rely on our PSP FW to do what it's designed to do. I don't
> > think it makes sense to mix and match here.
>
> Then you need to somehow tie the revoke list that's in the psp to the
> revoke list update logic we have. That's what we've done for hdcp2 (which
> is similarly to yours implemented in hw). The point is that on linux we
> now have a standard way to get these revoke lists updated/handled.
>
> I guess it wasn't clear how exactly I think you're supposed to combine
> them?
There's no driver sw required at all for our implementation and as far
as I know, HDCP 2.x requires that all of the key revoke handling be
handled in a secure processor rather than than on the host processor,
so I'm not sure how we make use if it. All the driver sw is
responsible for doing is saving/restoring the potentially updated srm
at suspend/resume/etc.
Alex
> -Daniel
>
>
> >
> > >>> For the defines I will create patches to use drm_hdcp where it is
> > >>> usable.
> > >>
> > >> Thanks a lot. Ime once we have shared definitions it's much easier to
> > >> also share some helpers, where it makes sense.
> > >>
> > >> Aside I think the hdcp code could also use a bit of demidlayering. At
> > >> least I'm not understanding why you add a 2nd abstraction layer for
> > >> i2c/dpcd, dm_helper already has that. That seems like one abstraction
> > >> layer too much.
> > >
> > > I haven't seen anything fly by or in the latest pull request ... you
> > > folks still working on this or more put on the "maybe, probably never"
> > > pile?
> > >
> >
> > Following up with Bhawan.
> >
> > Harry
> >
> > > -Daniel
> > >
> > >
> > >> -Daniel
> > >>
> > >>>
> > >>>
> > >>> Bhawan
> > >>>
> > >>> On 2019-10-09 2:43 p.m., Daniel Vetter wrote:
> > On Wed, Oct 9, 2019 at 8:23 PM Lakha, Bhawanpreet
> > wrote:
> > > Hi,
> > >
> > > The reason we don't use drm_hdcp is because our policy is to do hdcp
> > > verification using PSP/HW (onboard secure processor).
> > i915 also uses hw to auth, we still use the parts from drm_hdcp ...
> > Did you actually look at what's in there? It's essentially just shared
> > defines and data structures from the standard, plus a few minimal
> > helpers to en/decode some bits. Just from a quick read the entire
> > patch very much looks like midlayer everywhere design that we
> > discussed back when DC landed ...
> > -Daniel
> >
> > > Bhawan
> > >
> > > On 2019-10-09 12:32 p.m., Daniel Vetter wrote:
> > >> On Thu, Oct 03, 2019 at 11:08:03PM +0100, Colin Ian King wrote:
> > >>> Hi,
> > >>>
> > >>> Static analysis with Coverity has detected a potential issue with
> > >>> function validate_bksv in
> > >>> drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c with
> > >>> recent
> > >>> commit:
> > >>>
> > >>> commit ed9d8e2bcb003ec94658cafe9b1bb3960e2139ec
> > >>> Author: Bhawanpreet Lakha
> > >>> Date: Tue Aug 6 17:52:01 2019 -0400
> > >>>
> > >>> drm/amd/display: Add HDCP module
> > >> I think the real question here is ... why is this not using drm_hdcp?
> > >> -Daniel
> > >>
> > >>> The analysis is as follows:
> > >>>
> > >>>28 static inline enum mod_hdcp_status validate_bksv(struct
> > >>> mod_hdcp *hdcp)
> > >>>29 {
> > >>>
> > >>> CID 89852 (#1 of 1): Out-of-bounds read (OVERRUN)
> > >>>
> > >>> 1. overrun-local:
> > >>> Overrunning array of 5 bytes at byte offset 7 by dereferencing
> > >>> pointer
> > >>> (uint64_t *)hdcp->auth.msg.hdcp1.bksv.
> > >>>
> > >>>30uint64_t n = *(uint64_t *)hdcp->auth.msg.hdcp1.bksv;
> > >>>31uint8_t count = 0;
> > >>>32
> > >>>33while (n) {
> > >>>34count++;
> > >>>35n &= (n - 1);
> > >>>36}
> > >>>
> > >>> hdcp->auth.msg.hdcp1.bksv is an array of 5 uint8_t as defined in
> > >>> drivers/gpu/drm/amd/display/modules/hdcp/hdcp.h as follows:
> > >>>
> > >>> struct
Re: drm/amd/display: Add HDCP module - static analysis bug report
On Mon, Nov 04, 2019 at 03:23:09PM +, Harry Wentland wrote:
> On 2019-11-04 5:53 a.m., Daniel Vetter wrote:
> > On Wed, Oct 9, 2019 at 10:58 PM Daniel Vetter wrote:
> >> On Wed, Oct 9, 2019 at 10:46 PM Lakha, Bhawanpreet
> >> wrote:
> >>>
> >>> I misunderstood and was talking about the ksv validation specifically
> >>> (usage of drm_hdcp_check_ksvs_revoked()).
> >>
> >> Hm for that specifically I think you want to do both, i.e. both
> >> consult your psp, but also check for revoked ksvs with the core
> >> helper. At least on some platforms only the core helper might have the
> >> updated revoke list.
> >>
>
> I think it's an either/or. Either we use an HDCP implementation that's
> fully running in x86 kernel space (still not sure how that's compliant)
> or we fully rely on our PSP FW to do what it's designed to do. I don't
> think it makes sense to mix and match here.
Then you need to somehow tie the revoke list that's in the psp to the
revoke list update logic we have. That's what we've done for hdcp2 (which
is similarly to yours implemented in hw). The point is that on linux we
now have a standard way to get these revoke lists updated/handled.
I guess it wasn't clear how exactly I think you're supposed to combine
them?
-Daniel
>
> >>> For the defines I will create patches to use drm_hdcp where it is usable.
> >>
> >> Thanks a lot. Ime once we have shared definitions it's much easier to
> >> also share some helpers, where it makes sense.
> >>
> >> Aside I think the hdcp code could also use a bit of demidlayering. At
> >> least I'm not understanding why you add a 2nd abstraction layer for
> >> i2c/dpcd, dm_helper already has that. That seems like one abstraction
> >> layer too much.
> >
> > I haven't seen anything fly by or in the latest pull request ... you
> > folks still working on this or more put on the "maybe, probably never"
> > pile?
> >
>
> Following up with Bhawan.
>
> Harry
>
> > -Daniel
> >
> >
> >> -Daniel
> >>
> >>>
> >>>
> >>> Bhawan
> >>>
> >>> On 2019-10-09 2:43 p.m., Daniel Vetter wrote:
> On Wed, Oct 9, 2019 at 8:23 PM Lakha, Bhawanpreet
> wrote:
> > Hi,
> >
> > The reason we don't use drm_hdcp is because our policy is to do hdcp
> > verification using PSP/HW (onboard secure processor).
> i915 also uses hw to auth, we still use the parts from drm_hdcp ...
> Did you actually look at what's in there? It's essentially just shared
> defines and data structures from the standard, plus a few minimal
> helpers to en/decode some bits. Just from a quick read the entire
> patch very much looks like midlayer everywhere design that we
> discussed back when DC landed ...
> -Daniel
>
> > Bhawan
> >
> > On 2019-10-09 12:32 p.m., Daniel Vetter wrote:
> >> On Thu, Oct 03, 2019 at 11:08:03PM +0100, Colin Ian King wrote:
> >>> Hi,
> >>>
> >>> Static analysis with Coverity has detected a potential issue with
> >>> function validate_bksv in
> >>> drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c with recent
> >>> commit:
> >>>
> >>> commit ed9d8e2bcb003ec94658cafe9b1bb3960e2139ec
> >>> Author: Bhawanpreet Lakha
> >>> Date: Tue Aug 6 17:52:01 2019 -0400
> >>>
> >>> drm/amd/display: Add HDCP module
> >> I think the real question here is ... why is this not using drm_hdcp?
> >> -Daniel
> >>
> >>> The analysis is as follows:
> >>>
> >>>28 static inline enum mod_hdcp_status validate_bksv(struct
> >>> mod_hdcp *hdcp)
> >>>29 {
> >>>
> >>> CID 89852 (#1 of 1): Out-of-bounds read (OVERRUN)
> >>>
> >>> 1. overrun-local:
> >>> Overrunning array of 5 bytes at byte offset 7 by dereferencing pointer
> >>> (uint64_t *)hdcp->auth.msg.hdcp1.bksv.
> >>>
> >>>30uint64_t n = *(uint64_t *)hdcp->auth.msg.hdcp1.bksv;
> >>>31uint8_t count = 0;
> >>>32
> >>>33while (n) {
> >>>34count++;
> >>>35n &= (n - 1);
> >>>36}
> >>>
> >>> hdcp->auth.msg.hdcp1.bksv is an array of 5 uint8_t as defined in
> >>> drivers/gpu/drm/amd/display/modules/hdcp/hdcp.h as follows:
> >>>
> >>> struct mod_hdcp_message_hdcp1 {
> >>> uint8_t an[8];
> >>> uint8_t aksv[5];
> >>> uint8_t ainfo;
> >>> uint8_t bksv[5];
> >>> uint16_tr0p;
> >>> uint8_t bcaps;
> >>> uint16_tbstatus;
> >>> uint8_t ksvlist[635];
> >>> uint16_tksvlist_size;
> >>> uint8_t vp[20];
> >>>
> >>> uint16_tbinfo_dp;
> >>> };
> >>>
> >>> variable n is going to contain the contains of r0p and bcaps. I'm not
> >>> sure if that is intentional. If not, then the count is
Re: drm/amd/display: Add HDCP module - static analysis bug report
Hi Daniel,
I have the patches prepared but they needed some testing before I send them
(code needed a slight refactor to use the drm_hdcp.h), I should be able to send
the patches this week.
Thanks,
Bhawan
On 2019-11-04 10:23 a.m., Wentland, Harry wrote:
> On 2019-11-04 5:53 a.m., Daniel Vetter wrote:
>> On Wed, Oct 9, 2019 at 10:58 PM Daniel Vetter wrote:
>>> On Wed, Oct 9, 2019 at 10:46 PM Lakha, Bhawanpreet
>>> wrote:
I misunderstood and was talking about the ksv validation specifically
(usage of drm_hdcp_check_ksvs_revoked()).
>>> Hm for that specifically I think you want to do both, i.e. both
>>> consult your psp, but also check for revoked ksvs with the core
>>> helper. At least on some platforms only the core helper might have the
>>> updated revoke list.
>>>
> I think it's an either/or. Either we use an HDCP implementation that's
> fully running in x86 kernel space (still not sure how that's compliant)
> or we fully rely on our PSP FW to do what it's designed to do. I don't
> think it makes sense to mix and match here.
>
For the defines I will create patches to use drm_hdcp where it is usable.
>>> Thanks a lot. Ime once we have shared definitions it's much easier to
>>> also share some helpers, where it makes sense.
>>>
>>> Aside I think the hdcp code could also use a bit of demidlayering. At
>>> least I'm not understanding why you add a 2nd abstraction layer for
>>> i2c/dpcd, dm_helper already has that. That seems like one abstraction
>>> layer too much.
>> I haven't seen anything fly by or in the latest pull request ... you
>> folks still working on this or more put on the "maybe, probably never"
>> pile?
>>
> Following up with Bhawan.
>
> Harry
>
>> -Daniel
>>
>>
>>> -Daniel
>>>
Bhawan
On 2019-10-09 2:43 p.m., Daniel Vetter wrote:
> On Wed, Oct 9, 2019 at 8:23 PM Lakha, Bhawanpreet
> wrote:
>> Hi,
>>
>> The reason we don't use drm_hdcp is because our policy is to do hdcp
>> verification using PSP/HW (onboard secure processor).
> i915 also uses hw to auth, we still use the parts from drm_hdcp ...
> Did you actually look at what's in there? It's essentially just shared
> defines and data structures from the standard, plus a few minimal
> helpers to en/decode some bits. Just from a quick read the entire
> patch very much looks like midlayer everywhere design that we
> discussed back when DC landed ...
> -Daniel
>
>> Bhawan
>>
>> On 2019-10-09 12:32 p.m., Daniel Vetter wrote:
>>> On Thu, Oct 03, 2019 at 11:08:03PM +0100, Colin Ian King wrote:
Hi,
Static analysis with Coverity has detected a potential issue with
function validate_bksv in
drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c with recent
commit:
commit ed9d8e2bcb003ec94658cafe9b1bb3960e2139ec
Author: Bhawanpreet Lakha
Date: Tue Aug 6 17:52:01 2019 -0400
drm/amd/display: Add HDCP module
>>> I think the real question here is ... why is this not using drm_hdcp?
>>> -Daniel
>>>
The analysis is as follows:
28 static inline enum mod_hdcp_status validate_bksv(struct
mod_hdcp *hdcp)
29 {
CID 89852 (#1 of 1): Out-of-bounds read (OVERRUN)
1. overrun-local:
Overrunning array of 5 bytes at byte offset 7 by dereferencing pointer
(uint64_t *)hdcp->auth.msg.hdcp1.bksv.
30uint64_t n = *(uint64_t *)hdcp->auth.msg.hdcp1.bksv;
31uint8_t count = 0;
32
33while (n) {
34count++;
35n &= (n - 1);
36}
hdcp->auth.msg.hdcp1.bksv is an array of 5 uint8_t as defined in
drivers/gpu/drm/amd/display/modules/hdcp/hdcp.h as follows:
struct mod_hdcp_message_hdcp1 {
uint8_t an[8];
uint8_t aksv[5];
uint8_t ainfo;
uint8_t bksv[5];
uint16_tr0p;
uint8_t bcaps;
uint16_tbstatus;
uint8_t ksvlist[635];
uint16_tksvlist_size;
uint8_t vp[20];
uint16_tbinfo_dp;
};
variable n is going to contain the contains of r0p and bcaps. I'm not
sure if that is intentional. If not, then the count is going to be
incorrect if these are non-zero.
Colin
>> ___
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>>>
>>>
>>> --
>>> Daniel
Re: drm/amd/display: Add HDCP module - static analysis bug report
On 2019-11-04 5:53 a.m., Daniel Vetter wrote:
> On Wed, Oct 9, 2019 at 10:58 PM Daniel Vetter wrote:
>> On Wed, Oct 9, 2019 at 10:46 PM Lakha, Bhawanpreet
>> wrote:
>>>
>>> I misunderstood and was talking about the ksv validation specifically
>>> (usage of drm_hdcp_check_ksvs_revoked()).
>>
>> Hm for that specifically I think you want to do both, i.e. both
>> consult your psp, but also check for revoked ksvs with the core
>> helper. At least on some platforms only the core helper might have the
>> updated revoke list.
>>
I think it's an either/or. Either we use an HDCP implementation that's
fully running in x86 kernel space (still not sure how that's compliant)
or we fully rely on our PSP FW to do what it's designed to do. I don't
think it makes sense to mix and match here.
>>> For the defines I will create patches to use drm_hdcp where it is usable.
>>
>> Thanks a lot. Ime once we have shared definitions it's much easier to
>> also share some helpers, where it makes sense.
>>
>> Aside I think the hdcp code could also use a bit of demidlayering. At
>> least I'm not understanding why you add a 2nd abstraction layer for
>> i2c/dpcd, dm_helper already has that. That seems like one abstraction
>> layer too much.
>
> I haven't seen anything fly by or in the latest pull request ... you
> folks still working on this or more put on the "maybe, probably never"
> pile?
>
Following up with Bhawan.
Harry
> -Daniel
>
>
>> -Daniel
>>
>>>
>>>
>>> Bhawan
>>>
>>> On 2019-10-09 2:43 p.m., Daniel Vetter wrote:
On Wed, Oct 9, 2019 at 8:23 PM Lakha, Bhawanpreet
wrote:
> Hi,
>
> The reason we don't use drm_hdcp is because our policy is to do hdcp
> verification using PSP/HW (onboard secure processor).
i915 also uses hw to auth, we still use the parts from drm_hdcp ...
Did you actually look at what's in there? It's essentially just shared
defines and data structures from the standard, plus a few minimal
helpers to en/decode some bits. Just from a quick read the entire
patch very much looks like midlayer everywhere design that we
discussed back when DC landed ...
-Daniel
> Bhawan
>
> On 2019-10-09 12:32 p.m., Daniel Vetter wrote:
>> On Thu, Oct 03, 2019 at 11:08:03PM +0100, Colin Ian King wrote:
>>> Hi,
>>>
>>> Static analysis with Coverity has detected a potential issue with
>>> function validate_bksv in
>>> drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c with recent
>>> commit:
>>>
>>> commit ed9d8e2bcb003ec94658cafe9b1bb3960e2139ec
>>> Author: Bhawanpreet Lakha
>>> Date: Tue Aug 6 17:52:01 2019 -0400
>>>
>>> drm/amd/display: Add HDCP module
>> I think the real question here is ... why is this not using drm_hdcp?
>> -Daniel
>>
>>> The analysis is as follows:
>>>
>>>28 static inline enum mod_hdcp_status validate_bksv(struct mod_hdcp
>>> *hdcp)
>>>29 {
>>>
>>> CID 89852 (#1 of 1): Out-of-bounds read (OVERRUN)
>>>
>>> 1. overrun-local:
>>> Overrunning array of 5 bytes at byte offset 7 by dereferencing pointer
>>> (uint64_t *)hdcp->auth.msg.hdcp1.bksv.
>>>
>>>30uint64_t n = *(uint64_t *)hdcp->auth.msg.hdcp1.bksv;
>>>31uint8_t count = 0;
>>>32
>>>33while (n) {
>>>34count++;
>>>35n &= (n - 1);
>>>36}
>>>
>>> hdcp->auth.msg.hdcp1.bksv is an array of 5 uint8_t as defined in
>>> drivers/gpu/drm/amd/display/modules/hdcp/hdcp.h as follows:
>>>
>>> struct mod_hdcp_message_hdcp1 {
>>> uint8_t an[8];
>>> uint8_t aksv[5];
>>> uint8_t ainfo;
>>> uint8_t bksv[5];
>>> uint16_tr0p;
>>> uint8_t bcaps;
>>> uint16_tbstatus;
>>> uint8_t ksvlist[635];
>>> uint16_tksvlist_size;
>>> uint8_t vp[20];
>>>
>>> uint16_tbinfo_dp;
>>> };
>>>
>>> variable n is going to contain the contains of r0p and bcaps. I'm not
>>> sure if that is intentional. If not, then the count is going to be
>>> incorrect if these are non-zero.
>>>
>>> Colin
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>>
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm/amd/display: Add HDCP module - static analysis bug report
On Wed, Oct 9, 2019 at 10:58 PM Daniel Vetter wrote:
> On Wed, Oct 9, 2019 at 10:46 PM Lakha, Bhawanpreet
> wrote:
> >
> > I misunderstood and was talking about the ksv validation specifically
> > (usage of drm_hdcp_check_ksvs_revoked()).
>
> Hm for that specifically I think you want to do both, i.e. both
> consult your psp, but also check for revoked ksvs with the core
> helper. At least on some platforms only the core helper might have the
> updated revoke list.
>
> > For the defines I will create patches to use drm_hdcp where it is usable.
>
> Thanks a lot. Ime once we have shared definitions it's much easier to
> also share some helpers, where it makes sense.
>
> Aside I think the hdcp code could also use a bit of demidlayering. At
> least I'm not understanding why you add a 2nd abstraction layer for
> i2c/dpcd, dm_helper already has that. That seems like one abstraction
> layer too much.
I haven't seen anything fly by or in the latest pull request ... you
folks still working on this or more put on the "maybe, probably never"
pile?
-Daniel
> -Daniel
>
> >
> >
> > Bhawan
> >
> > On 2019-10-09 2:43 p.m., Daniel Vetter wrote:
> > > On Wed, Oct 9, 2019 at 8:23 PM Lakha, Bhawanpreet
> > > wrote:
> > >> Hi,
> > >>
> > >> The reason we don't use drm_hdcp is because our policy is to do hdcp
> > >> verification using PSP/HW (onboard secure processor).
> > > i915 also uses hw to auth, we still use the parts from drm_hdcp ...
> > > Did you actually look at what's in there? It's essentially just shared
> > > defines and data structures from the standard, plus a few minimal
> > > helpers to en/decode some bits. Just from a quick read the entire
> > > patch very much looks like midlayer everywhere design that we
> > > discussed back when DC landed ...
> > > -Daniel
> > >
> > >> Bhawan
> > >>
> > >> On 2019-10-09 12:32 p.m., Daniel Vetter wrote:
> > >>> On Thu, Oct 03, 2019 at 11:08:03PM +0100, Colin Ian King wrote:
> > Hi,
> >
> > Static analysis with Coverity has detected a potential issue with
> > function validate_bksv in
> > drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c with recent
> > commit:
> >
> > commit ed9d8e2bcb003ec94658cafe9b1bb3960e2139ec
> > Author: Bhawanpreet Lakha
> > Date: Tue Aug 6 17:52:01 2019 -0400
> >
> > drm/amd/display: Add HDCP module
> > >>> I think the real question here is ... why is this not using drm_hdcp?
> > >>> -Daniel
> > >>>
> > The analysis is as follows:
> >
> > 28 static inline enum mod_hdcp_status validate_bksv(struct mod_hdcp
> > *hdcp)
> > 29 {
> >
> > CID 89852 (#1 of 1): Out-of-bounds read (OVERRUN)
> >
> > 1. overrun-local:
> > Overrunning array of 5 bytes at byte offset 7 by dereferencing pointer
> > (uint64_t *)hdcp->auth.msg.hdcp1.bksv.
> >
> > 30uint64_t n = *(uint64_t *)hdcp->auth.msg.hdcp1.bksv;
> > 31uint8_t count = 0;
> > 32
> > 33while (n) {
> > 34count++;
> > 35n &= (n - 1);
> > 36}
> >
> > hdcp->auth.msg.hdcp1.bksv is an array of 5 uint8_t as defined in
> > drivers/gpu/drm/amd/display/modules/hdcp/hdcp.h as follows:
> >
> > struct mod_hdcp_message_hdcp1 {
> > uint8_t an[8];
> > uint8_t aksv[5];
> > uint8_t ainfo;
> > uint8_t bksv[5];
> > uint16_tr0p;
> > uint8_t bcaps;
> > uint16_tbstatus;
> > uint8_t ksvlist[635];
> > uint16_tksvlist_size;
> > uint8_t vp[20];
> >
> > uint16_tbinfo_dp;
> > };
> >
> > variable n is going to contain the contains of r0p and bcaps. I'm not
> > sure if that is intentional. If not, then the count is going to be
> > incorrect if these are non-zero.
> >
> > Colin
> > >> ___
> > >> dri-devel mailing list
> > >> dri-devel@lists.freedesktop.org
> > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > >
> > >
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm/amd/display: Add HDCP module - static analysis bug report
On Wed, Oct 9, 2019 at 10:46 PM Lakha, Bhawanpreet
wrote:
>
> I misunderstood and was talking about the ksv validation specifically
> (usage of drm_hdcp_check_ksvs_revoked()).
Hm for that specifically I think you want to do both, i.e. both
consult your psp, but also check for revoked ksvs with the core
helper. At least on some platforms only the core helper might have the
updated revoke list.
> For the defines I will create patches to use drm_hdcp where it is usable.
Thanks a lot. Ime once we have shared definitions it's much easier to
also share some helpers, where it makes sense.
Aside I think the hdcp code could also use a bit of demidlayering. At
least I'm not understanding why you add a 2nd abstraction layer for
i2c/dpcd, dm_helper already has that. That seems like one abstraction
layer too much.
-Daniel
>
>
> Bhawan
>
> On 2019-10-09 2:43 p.m., Daniel Vetter wrote:
> > On Wed, Oct 9, 2019 at 8:23 PM Lakha, Bhawanpreet
> > wrote:
> >> Hi,
> >>
> >> The reason we don't use drm_hdcp is because our policy is to do hdcp
> >> verification using PSP/HW (onboard secure processor).
> > i915 also uses hw to auth, we still use the parts from drm_hdcp ...
> > Did you actually look at what's in there? It's essentially just shared
> > defines and data structures from the standard, plus a few minimal
> > helpers to en/decode some bits. Just from a quick read the entire
> > patch very much looks like midlayer everywhere design that we
> > discussed back when DC landed ...
> > -Daniel
> >
> >> Bhawan
> >>
> >> On 2019-10-09 12:32 p.m., Daniel Vetter wrote:
> >>> On Thu, Oct 03, 2019 at 11:08:03PM +0100, Colin Ian King wrote:
> Hi,
>
> Static analysis with Coverity has detected a potential issue with
> function validate_bksv in
> drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c with recent
> commit:
>
> commit ed9d8e2bcb003ec94658cafe9b1bb3960e2139ec
> Author: Bhawanpreet Lakha
> Date: Tue Aug 6 17:52:01 2019 -0400
>
> drm/amd/display: Add HDCP module
> >>> I think the real question here is ... why is this not using drm_hdcp?
> >>> -Daniel
> >>>
> The analysis is as follows:
>
> 28 static inline enum mod_hdcp_status validate_bksv(struct mod_hdcp
> *hdcp)
> 29 {
>
> CID 89852 (#1 of 1): Out-of-bounds read (OVERRUN)
>
> 1. overrun-local:
> Overrunning array of 5 bytes at byte offset 7 by dereferencing pointer
> (uint64_t *)hdcp->auth.msg.hdcp1.bksv.
>
> 30uint64_t n = *(uint64_t *)hdcp->auth.msg.hdcp1.bksv;
> 31uint8_t count = 0;
> 32
> 33while (n) {
> 34count++;
> 35n &= (n - 1);
> 36}
>
> hdcp->auth.msg.hdcp1.bksv is an array of 5 uint8_t as defined in
> drivers/gpu/drm/amd/display/modules/hdcp/hdcp.h as follows:
>
> struct mod_hdcp_message_hdcp1 {
> uint8_t an[8];
> uint8_t aksv[5];
> uint8_t ainfo;
> uint8_t bksv[5];
> uint16_tr0p;
> uint8_t bcaps;
> uint16_tbstatus;
> uint8_t ksvlist[635];
> uint16_tksvlist_size;
> uint8_t vp[20];
>
> uint16_tbinfo_dp;
> };
>
> variable n is going to contain the contains of r0p and bcaps. I'm not
> sure if that is intentional. If not, then the count is going to be
> incorrect if these are non-zero.
>
> Colin
> >> ___
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> >
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm/amd/display: Add HDCP module - static analysis bug report
I misunderstood and was talking about the ksv validation specifically
(usage of drm_hdcp_check_ksvs_revoked()).
For the defines I will create patches to use drm_hdcp where it is usable.
Bhawan
On 2019-10-09 2:43 p.m., Daniel Vetter wrote:
> On Wed, Oct 9, 2019 at 8:23 PM Lakha, Bhawanpreet
> wrote:
>> Hi,
>>
>> The reason we don't use drm_hdcp is because our policy is to do hdcp
>> verification using PSP/HW (onboard secure processor).
> i915 also uses hw to auth, we still use the parts from drm_hdcp ...
> Did you actually look at what's in there? It's essentially just shared
> defines and data structures from the standard, plus a few minimal
> helpers to en/decode some bits. Just from a quick read the entire
> patch very much looks like midlayer everywhere design that we
> discussed back when DC landed ...
> -Daniel
>
>> Bhawan
>>
>> On 2019-10-09 12:32 p.m., Daniel Vetter wrote:
>>> On Thu, Oct 03, 2019 at 11:08:03PM +0100, Colin Ian King wrote:
Hi,
Static analysis with Coverity has detected a potential issue with
function validate_bksv in
drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c with recent
commit:
commit ed9d8e2bcb003ec94658cafe9b1bb3960e2139ec
Author: Bhawanpreet Lakha
Date: Tue Aug 6 17:52:01 2019 -0400
drm/amd/display: Add HDCP module
>>> I think the real question here is ... why is this not using drm_hdcp?
>>> -Daniel
>>>
The analysis is as follows:
28 static inline enum mod_hdcp_status validate_bksv(struct mod_hdcp
*hdcp)
29 {
CID 89852 (#1 of 1): Out-of-bounds read (OVERRUN)
1. overrun-local:
Overrunning array of 5 bytes at byte offset 7 by dereferencing pointer
(uint64_t *)hdcp->auth.msg.hdcp1.bksv.
30uint64_t n = *(uint64_t *)hdcp->auth.msg.hdcp1.bksv;
31uint8_t count = 0;
32
33while (n) {
34count++;
35n &= (n - 1);
36}
hdcp->auth.msg.hdcp1.bksv is an array of 5 uint8_t as defined in
drivers/gpu/drm/amd/display/modules/hdcp/hdcp.h as follows:
struct mod_hdcp_message_hdcp1 {
uint8_t an[8];
uint8_t aksv[5];
uint8_t ainfo;
uint8_t bksv[5];
uint16_tr0p;
uint8_t bcaps;
uint16_tbstatus;
uint8_t ksvlist[635];
uint16_tksvlist_size;
uint8_t vp[20];
uint16_tbinfo_dp;
};
variable n is going to contain the contains of r0p and bcaps. I'm not
sure if that is intentional. If not, then the count is going to be
incorrect if these are non-zero.
Colin
>> ___
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm/amd/display: Add HDCP module - static analysis bug report
On Wed, Oct 9, 2019 at 8:23 PM Lakha, Bhawanpreet
wrote:
>
> Hi,
>
> The reason we don't use drm_hdcp is because our policy is to do hdcp
> verification using PSP/HW (onboard secure processor).
i915 also uses hw to auth, we still use the parts from drm_hdcp ...
Did you actually look at what's in there? It's essentially just shared
defines and data structures from the standard, plus a few minimal
helpers to en/decode some bits. Just from a quick read the entire
patch very much looks like midlayer everywhere design that we
discussed back when DC landed ...
-Daniel
>
> Bhawan
>
> On 2019-10-09 12:32 p.m., Daniel Vetter wrote:
> > On Thu, Oct 03, 2019 at 11:08:03PM +0100, Colin Ian King wrote:
> >> Hi,
> >>
> >> Static analysis with Coverity has detected a potential issue with
> >> function validate_bksv in
> >> drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c with recent
> >> commit:
> >>
> >> commit ed9d8e2bcb003ec94658cafe9b1bb3960e2139ec
> >> Author: Bhawanpreet Lakha
> >> Date: Tue Aug 6 17:52:01 2019 -0400
> >>
> >> drm/amd/display: Add HDCP module
> > I think the real question here is ... why is this not using drm_hdcp?
> > -Daniel
> >
> >>
> >> The analysis is as follows:
> >>
> >> 28 static inline enum mod_hdcp_status validate_bksv(struct mod_hdcp
> >> *hdcp)
> >> 29 {
> >>
> >> CID 89852 (#1 of 1): Out-of-bounds read (OVERRUN)
> >>
> >> 1. overrun-local:
> >> Overrunning array of 5 bytes at byte offset 7 by dereferencing pointer
> >> (uint64_t *)hdcp->auth.msg.hdcp1.bksv.
> >>
> >> 30uint64_t n = *(uint64_t *)hdcp->auth.msg.hdcp1.bksv;
> >> 31uint8_t count = 0;
> >> 32
> >> 33while (n) {
> >> 34count++;
> >> 35n &= (n - 1);
> >> 36}
> >>
> >> hdcp->auth.msg.hdcp1.bksv is an array of 5 uint8_t as defined in
> >> drivers/gpu/drm/amd/display/modules/hdcp/hdcp.h as follows:
> >>
> >> struct mod_hdcp_message_hdcp1 {
> >> uint8_t an[8];
> >> uint8_t aksv[5];
> >> uint8_t ainfo;
> >> uint8_t bksv[5];
> >> uint16_tr0p;
> >> uint8_t bcaps;
> >> uint16_tbstatus;
> >> uint8_t ksvlist[635];
> >> uint16_tksvlist_size;
> >> uint8_t vp[20];
> >>
> >> uint16_tbinfo_dp;
> >> };
> >>
> >> variable n is going to contain the contains of r0p and bcaps. I'm not
> >> sure if that is intentional. If not, then the count is going to be
> >> incorrect if these are non-zero.
> >>
> >> Colin
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm/amd/display: Add HDCP module - static analysis bug report
Hi,
The reason we don't use drm_hdcp is because our policy is to do hdcp
verification using PSP/HW (onboard secure processor).
Bhawan
On 2019-10-09 12:32 p.m., Daniel Vetter wrote:
> On Thu, Oct 03, 2019 at 11:08:03PM +0100, Colin Ian King wrote:
>> Hi,
>>
>> Static analysis with Coverity has detected a potential issue with
>> function validate_bksv in
>> drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c with recent
>> commit:
>>
>> commit ed9d8e2bcb003ec94658cafe9b1bb3960e2139ec
>> Author: Bhawanpreet Lakha
>> Date: Tue Aug 6 17:52:01 2019 -0400
>>
>> drm/amd/display: Add HDCP module
> I think the real question here is ... why is this not using drm_hdcp?
> -Daniel
>
>>
>> The analysis is as follows:
>>
>> 28 static inline enum mod_hdcp_status validate_bksv(struct mod_hdcp *hdcp)
>> 29 {
>>
>> CID 89852 (#1 of 1): Out-of-bounds read (OVERRUN)
>>
>> 1. overrun-local:
>> Overrunning array of 5 bytes at byte offset 7 by dereferencing pointer
>> (uint64_t *)hdcp->auth.msg.hdcp1.bksv.
>>
>> 30uint64_t n = *(uint64_t *)hdcp->auth.msg.hdcp1.bksv;
>> 31uint8_t count = 0;
>> 32
>> 33while (n) {
>> 34count++;
>> 35n &= (n - 1);
>> 36}
>>
>> hdcp->auth.msg.hdcp1.bksv is an array of 5 uint8_t as defined in
>> drivers/gpu/drm/amd/display/modules/hdcp/hdcp.h as follows:
>>
>> struct mod_hdcp_message_hdcp1 {
>> uint8_t an[8];
>> uint8_t aksv[5];
>> uint8_t ainfo;
>> uint8_t bksv[5];
>> uint16_tr0p;
>> uint8_t bcaps;
>> uint16_tbstatus;
>> uint8_t ksvlist[635];
>> uint16_tksvlist_size;
>> uint8_t vp[20];
>>
>> uint16_tbinfo_dp;
>> };
>>
>> variable n is going to contain the contains of r0p and bcaps. I'm not
>> sure if that is intentional. If not, then the count is going to be
>> incorrect if these are non-zero.
>>
>> Colin
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm/amd/display: Add HDCP module - static analysis bug report
On Thu, Oct 03, 2019 at 11:08:03PM +0100, Colin Ian King wrote:
> Hi,
>
> Static analysis with Coverity has detected a potential issue with
> function validate_bksv in
> drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c with recent
> commit:
>
> commit ed9d8e2bcb003ec94658cafe9b1bb3960e2139ec
> Author: Bhawanpreet Lakha
> Date: Tue Aug 6 17:52:01 2019 -0400
>
> drm/amd/display: Add HDCP module
I think the real question here is ... why is this not using drm_hdcp?
-Daniel
>
>
> The analysis is as follows:
>
> 28 static inline enum mod_hdcp_status validate_bksv(struct mod_hdcp *hdcp)
> 29 {
>
> CID 89852 (#1 of 1): Out-of-bounds read (OVERRUN)
>
> 1. overrun-local:
> Overrunning array of 5 bytes at byte offset 7 by dereferencing pointer
> (uint64_t *)hdcp->auth.msg.hdcp1.bksv.
>
> 30uint64_t n = *(uint64_t *)hdcp->auth.msg.hdcp1.bksv;
> 31uint8_t count = 0;
> 32
> 33while (n) {
> 34count++;
> 35n &= (n - 1);
> 36}
>
> hdcp->auth.msg.hdcp1.bksv is an array of 5 uint8_t as defined in
> drivers/gpu/drm/amd/display/modules/hdcp/hdcp.h as follows:
>
> struct mod_hdcp_message_hdcp1 {
> uint8_t an[8];
> uint8_t aksv[5];
> uint8_t ainfo;
> uint8_t bksv[5];
> uint16_tr0p;
> uint8_t bcaps;
> uint16_tbstatus;
> uint8_t ksvlist[635];
> uint16_tksvlist_size;
> uint8_t vp[20];
>
> uint16_tbinfo_dp;
> };
>
> variable n is going to contain the contains of r0p and bcaps. I'm not
> sure if that is intentional. If not, then the count is going to be
> incorrect if these are non-zero.
>
> Colin
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
re: drm/amd/display: Add HDCP module - static analysis bug report
Hi,
Static analysis with Coverity has detected a potential issue with
function validate_bksv in
drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c with recent
commit:
commit ed9d8e2bcb003ec94658cafe9b1bb3960e2139ec
Author: Bhawanpreet Lakha
Date: Tue Aug 6 17:52:01 2019 -0400
drm/amd/display: Add HDCP module
The analysis is as follows:
28 static inline enum mod_hdcp_status validate_bksv(struct mod_hdcp *hdcp)
29 {
CID 89852 (#1 of 1): Out-of-bounds read (OVERRUN)
1. overrun-local:
Overrunning array of 5 bytes at byte offset 7 by dereferencing pointer
(uint64_t *)hdcp->auth.msg.hdcp1.bksv.
30uint64_t n = *(uint64_t *)hdcp->auth.msg.hdcp1.bksv;
31uint8_t count = 0;
32
33while (n) {
34count++;
35n &= (n - 1);
36}
hdcp->auth.msg.hdcp1.bksv is an array of 5 uint8_t as defined in
drivers/gpu/drm/amd/display/modules/hdcp/hdcp.h as follows:
struct mod_hdcp_message_hdcp1 {
uint8_t an[8];
uint8_t aksv[5];
uint8_t ainfo;
uint8_t bksv[5];
uint16_tr0p;
uint8_t bcaps;
uint16_tbstatus;
uint8_t ksvlist[635];
uint16_tksvlist_size;
uint8_t vp[20];
uint16_tbinfo_dp;
};
variable n is going to contain the contains of r0p and bcaps. I'm not
sure if that is intentional. If not, then the count is going to be
incorrect if these are non-zero.
Colin
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
