[PATCH envytools] nvbios: Add missing null byte to string read from file.

[PATCH envytools] nvbios: Add missing null byte to string read from file.

[Christian Costa]

---
 nvbios/nvbios.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/nvbios/nvbios.c b/nvbios/nvbios.c
index f7aafe3..28e62ad 100644
--- a/nvbios/nvbios.c
+++ b/nvbios/nvbios.c
@@ -774,11 +774,12 @@ int set_strap_from_string(const char* strap_s)
 int set_strap_from_file(const char *path)
 {
 	FILE *strapfile = NULL;
-	char tmp[21];
+	char tmp[22];
 
 	strapfile = fopen(path, "r");
 	if (strapfile) {
-		fread(tmp, 1, 21, strapfile);
+		size_t size = fread(tmp, 1, 21, strapfile);
+		tmp[size] = 0;
 		return set_strap_from_string(tmp);
 	}
 
-- 
1.9.1

[PATCH envytools] nva: Add nvaforcetemp to git ignore and sort it for better update

[Christian Costa]

---
 nva/.gitignore | 53 +++++++++++++++++++++++++++--------------------------
 1 file changed, 27 insertions(+), 26 deletions(-)

diff --git a/nva/.gitignore b/nva/.gitignore
index 5c9184b..72de056 100644
--- a/nva/.gitignore
+++ b/nva/.gitignore
@@ -1,39 +1,40 @@
+evotiming
+libnva.a
+libnva.so
+nv01dump
+nv01post
+nvacounter
+nvadownload
+nvaevo
+nvafakebios
 nvafill
+nvaforcetemp
+nvafucstart
+nvafuzz
+nvagetbios
+nvagetpmu
 nvahammer
 nvalist
+nvamemtiming
+nvammiotracereplay
 nvapeek
+nvapeeki
 nvapeekstat
+nvapms
 nvapoke
-nvapeeki
-nvagetbios
-nvafakebios
+nvapy.c
+nvapy.so
 nvascan
+nvaspyi2c
+nvastrscan
 nvatiming
-nvapms
-nvafuzz
+nvaupload
+nvavucstart
 nvawatch
-nvacounter
-nvastrscan
-nvaevo
-nvaxttime
-nvaxtstart
+nvaxtinsn
 nvaxtpeek
 nvaxtpoke
 nvaxtrsr
 nvaxtssr
-nvaxtinsn
-nvaupload
-nvadownload
-evotiming
-nvavucstart
-nvafucstart
-nvamemtiming
-nv01post
-nv01dump
-nvammiotracereplay
-nvaspyi2c
-libnva.a
-libnva.so
-nvapy.c
-nvapy.so
-nvagetpmu
+nvaxtstart
+nvaxttime
-- 
1.9.1

Re: [PATCH envytools] nva: Add nvaforcetemp to git ignore and sort it for better update

[Martin Peres]

Woops, my bad... Sorry!

[PATCH envytools] nvamemtiming: Update usage.

[Christian Costa]

---
 nva/nvamemtiming.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/nva/nvamemtiming.c b/nva/nvamemtiming.c
index 4468f81..75c6e49 100644
--- a/nva/nvamemtiming.c
+++ b/nva/nvamemtiming.c
@@ -51,6 +51,8 @@ void usage(int argc, char **argv)
 	fprintf(stderr, "\t-e: Only modify the specified index (to be used with -v)\n");
 	fprintf(stderr, "\t-v: Set the specified value to the specified entry\n");
 	fprintf(stderr, "\t-b index: Consider the specified index as a bitfield and RE it\n");
+	fprintf(stderr, "\t-s index: Specify range start in timing entry (Deep & Shallow modes)\n");
+	fprintf(stderr, "\t-f index: Specify range end in timing entry (Deep & Shallow modes)\n");
 	fprintf(stderr, "\t-d timing_entry_high: For each indexes, iterate between the timing_entry and the timing_entry_high value (Deep mode)\n");
 	exit(-1);
 }
-- 
1.9.1

[PATCH envytools] nvamemtiming: Handle target < initial case when iterating values

[Christian Costa]

Otherwise some values are not tested at all.
---
 nva/set_timings.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/nva/set_timings.c b/nva/set_timings.c
index 6cd831c..7a8f845 100644
--- a/nva/set_timings.c
+++ b/nva/set_timings.c
@@ -408,7 +408,7 @@ static void
 iterate_values(struct nvamemtiming_conf *conf, FILE *outf, uint8_t index, enum color color)
 {
 	uint8_t initial, target;
-	int v;
+	int v, incr;
 
 	if (conf->mode != MODE_DEEP)
 		return;
@@ -416,7 +416,8 @@ iterate_values(struct nvamemtiming_conf *conf, FILE *outf, uint8_t index, enum c
 	initial = conf->vbios.data[conf->vbios.timing_entry_offset + index];
 	target = conf->vbios.data[conf->deep.timing_entry_offset + index];
 
-	for (v = initial+1; v <= target; v++) {
+	incr = target > initial ? 1 : -1;
+	for (v = initial+incr; v != target+incr; v+=incr) {
 		conf->vbios.data[conf->vbios.timing_entry_offset + index] = v;
 		launch(conf, outf, index + 1 , COLOR);
 	}
-- 
1.9.1

Re: [PATCH envytools] nvamemtiming: Handle target < initial case when iterating values

[Martin Peres]

On 31/08/2014 15:00, Christian Costa wrote:
> Otherwise some values are not tested at all.
I would rather have a warning than the program doing stuff behind my 
back. This is a dev tool, dumb == good ;)

Re: [PATCH envytools] nvamemtiming: Handle target < initial case when iterating values

[Christian Costa]

Le 31/08/2014 15:12, Martin Peres a écrit :
> On 31/08/2014 15:00, Christian Costa wrote:
>> Otherwise some values are not tested at all.
> I would rather have a warning than the program doing stuff behind my 
> back. This is a dev tool, dumb == good ;)
>
But why didn't you call it Dumb mode rather that Deep mode. That way, I 
wouldn't have tried to use it and figure out what it does. ;p

So about my problem,

Command:
sudo nvamemtiming pramin 0x6a22 3 0 -d 5

Vbios info:
Table offset at 0x6a22
Entry 3 => 0x6a73: 0d 06 0f 26 00 45 00 1b 00 0b 0d 08 04 0c 01 00 00 00 
03 03 10 0a 09 00 03
Entry 5 => 0x6aa5: 05 04 08 05 00 09 00 03 00 02 02 02 02 0c 02 00 00 00 
03 03 08 0a 0f 00 02

With the current code, I get these iterations:
timing table at: 6a22
Deep mode: Will iterate between 5 and 24
idx 5, initial 45, target 9
idx 7, initial 1b, target 3
idx 9, initial b, target 2
idx 10, initial d, target 2
idx 11, initial 8, target 2
idx 12, initial 4, target 2
idx 13, initial c, target c
idx 18, initial 3, target 3
idx 19, initial 3, target 3
idx 20, initial 10, target 8
idx 21, initial a, target a
idx 22, initial 9, target f => a b c d e f
idx 24, initial 3, target 2

It does not test much for the reason most of the time the target value 
is lower than the initial one.

With my patch:
timing table at: 6a22
Deep mode: Will iterate between 5 and 24
idx 5, initial 45, target 9 => 44 43 42 41 40 3f 3e 3d 3c 3b 3a 39 38 37 
36 35 34 33 32 31 30 2f 2e 2d 2c 2b 2a 29 28 27 26 25 24 23 22 21 20 1f 
1e 1d 1c 1b 1a 19 18 17 16 15 14 13 12 11 10 f e d c b a 9
idx 7, initial 1b, target 3 => 1a 19 18 17 16 15 14 13 12 11 10 f e d c 
b a 9 8 7 6 5 4 3
idx 9, initial b, target 2 => a 9 8 7 6 5 4 3 2
idx 10, initial d, target 2 => c b a 9 8 7 6 5 4 3 2
idx 11, initial 8, target 2 => 7 6 5 4 3 2
idx 12, initial 4, target 2 => 3 2
idx 13, initial c, target c
idx 18, initial 3, target 3
idx 19, initial 3, target 3
idx 20, initial 10, target 8 => f e d c b a 9 8
idx 21, initial a, target a
idx 22, initial 9, target f => a b c d e f
idx 24, initial 3, target 2 =>2

Maybe shallow mode would be enough to test timing but what the purpose 
of deep mode in that case ?

Christian

Re: [PATCH envytools] nvamemtiming: Handle target < initial case when iterating values

[Martin Peres]

On 31/08/2014 16:01, Christian Costa wrote:
> Le 31/08/2014 15:12, Martin Peres a écrit :
>> On 31/08/2014 15:00, Christian Costa wrote:
>>> Otherwise some values are not tested at all.
>> I would rather have a warning than the program doing stuff behind my 
>> back. This is a dev tool, dumb == good ;)
>>
> But why didn't you call it Dumb mode rather that Deep mode. That way, 
> I wouldn't have tried to use it and figure out what it does. ;p

Target is supposed to be higher than initial! Initial == minimum safe 
timing.

You just reversed the order, try:
sudo nvamemtiming pramin 0x6a22 5 0 -d 3

Re: [PATCH envytools] nvamemtiming: Handle target < initial case when iterating values

[Christian Costa]

Le 31/08/2014 16:11, Martin Peres a écrit :
> On 31/08/2014 16:01, Christian Costa wrote:
>> Le 31/08/2014 15:12, Martin Peres a écrit :
>>> On 31/08/2014 15:00, Christian Costa wrote:
>>>> Otherwise some values are not tested at all.
>>> I would rather have a warning than the program doing stuff behind my 
>>> back. This is a dev tool, dumb == good ;)
>>>
>> But why didn't you call it Dumb mode rather that Deep mode. That way, 
>> I wouldn't have tried to use it and figure out what it does. ;p
>
> Target is supposed to be higher than initial! Initial == minimum safe 
> timing.
>
> You just reversed the order, try:
> sudo nvamemtiming pramin 0x6a22 5 0 -d 3
>
Ok. It's intended then. Thanks!

Re: [PATCH envytools] nvamemtiming: Handle target < initial case when iterating values

[Martin Peres]

On 31/08/2014 16:19, Christian Costa wrote:
> Le 31/08/2014 16:11, Martin Peres a écrit :
>> On 31/08/2014 16:01, Christian Costa wrote:
>>> Le 31/08/2014 15:12, Martin Peres a écrit :
>>>> On 31/08/2014 15:00, Christian Costa wrote:
>>>>> Otherwise some values are not tested at all.
>>>> I would rather have a warning than the program doing stuff behind 
>>>> my back. This is a dev tool, dumb == good ;)
>>>>
>>> But why didn't you call it Dumb mode rather that Deep mode. That 
>>> way, I wouldn't have tried to use it and figure out what it does. ;p
>>
>> Target is supposed to be higher than initial! Initial == minimum safe 
>> timing.
>>
>> You just reversed the order, try:
>> sudo nvamemtiming pramin 0x6a22 5 0 -d 3
>>
> Ok. It's intended then. Thanks!

I totally agree that this tool is a little confusing. If you have 
suggestions on how to improve the usage doc, feel free to propose me 
something better :)

fprintf(stderr, "\t-d timing_entry_high: For each indexes, iterate 
between the timing_entry and the timing_entry_high value (Deep mode)\n");

Speaking about this, this should be corrected to "For each index", no 
plural after each. I'll fix it right now.

Re: [PATCH envytools] nvamemtiming: Handle target < initial case when iterating values

[Christian Costa]

Le 31/08/2014 16:23, Martin Peres a écrit :
> On 31/08/2014 16:19, Christian Costa wrote:
>> Le 31/08/2014 16:11, Martin Peres a écrit :
>>> On 31/08/2014 16:01, Christian Costa wrote:
>>>> Le 31/08/2014 15:12, Martin Peres a écrit :
>>>>> On 31/08/2014 15:00, Christian Costa wrote:
>>>>>> Otherwise some values are not tested at all.
>>>>> I would rather have a warning than the program doing stuff behind 
>>>>> my back. This is a dev tool, dumb == good ;)
>>>>>
>>>> But why didn't you call it Dumb mode rather that Deep mode. That 
>>>> way, I wouldn't have tried to use it and figure out what it does. ;p
>>>
>>> Target is supposed to be higher than initial! Initial == minimum 
>>> safe timing.
>>>
>>> You just reversed the order, try:
>>> sudo nvamemtiming pramin 0x6a22 5 0 -d 3
>>>
>> Ok. It's intended then. Thanks!
>
> I totally agree that this tool is a little confusing. If you have 
> suggestions on how to improve the usage doc, feel free to propose me 
> something better :)
>
> fprintf(stderr, "\t-d timing_entry_high: For each indexes, iterate 
> between the timing_entry and the timing_entry_high value (Deep mode)\n");
>
> Speaking about this, this should be corrected to "For each index", no 
> plural after each. I'll fix it right now.
>
Ok. I will think about it.
Some comments in the source code can help also to know what it does and 
what it is used for. Same for README.

Re: [PATCH envytools] nvamemtiming: Handle target < initial case when iterating values

[Christian Costa]

Le 31/08/2014 17:18, Christian Costa a écrit :
> Le 31/08/2014 16:23, Martin Peres a écrit :
>> On 31/08/2014 16:19, Christian Costa wrote:
>>> Le 31/08/2014 16:11, Martin Peres a écrit :
>>>> On 31/08/2014 16:01, Christian Costa wrote:
>>>>> Le 31/08/2014 15:12, Martin Peres a écrit :
>>>>>> On 31/08/2014 15:00, Christian Costa wrote:
>>>>>>> Otherwise some values are not tested at all.
>>>>>> I would rather have a warning than the program doing stuff behind 
>>>>>> my back. This is a dev tool, dumb == good ;)
>>>>>>
>>>>> But why didn't you call it Dumb mode rather that Deep mode. That 
>>>>> way, I wouldn't have tried to use it and figure out what it does. ;p
>>>>
>>>> Target is supposed to be higher than initial! Initial == minimum 
>>>> safe timing.
>>>>
>>>> You just reversed the order, try:
>>>> sudo nvamemtiming pramin 0x6a22 5 0 -d 3
>>>>
>>> Ok. It's intended then. Thanks!
>>
>> I totally agree that this tool is a little confusing. If you have 
>> suggestions on how to improve the usage doc, feel free to propose me 
>> something better :)
>>
>> fprintf(stderr, "\t-d timing_entry_high: For each indexes, iterate 
>> between the timing_entry and the timing_entry_high value (Deep 
>> mode)\n");
>>
>> Speaking about this, this should be corrected to "For each index", no 
>> plural after each. I'll fix it right now.
>>
> Ok. I will think about it.
> Some comments in the source code can help also to know what it does 
> and what it is used for. Same for README.
Btw, the plural of index is indices.

Re: [PATCH envytools] nvamemtiming: Handle target < initial case when iterating values

[Martin Peres]

On 31/08/2014 17:20, Christian Costa wrote:
> Le 31/08/2014 17:18, Christian Costa a écrit :
>> Le 31/08/2014 16:23, Martin Peres a écrit :
>>> On 31/08/2014 16:19, Christian Costa wrote:
>>>> Le 31/08/2014 16:11, Martin Peres a écrit :
>>>>> On 31/08/2014 16:01, Christian Costa wrote:
>>>>>> Le 31/08/2014 15:12, Martin Peres a écrit :
>>>>>>> On 31/08/2014 15:00, Christian Costa wrote:
>>>>>>>> Otherwise some values are not tested at all.
>>>>>>> I would rather have a warning than the program doing stuff 
>>>>>>> behind my back. This is a dev tool, dumb == good ;)
>>>>>>>
>>>>>> But why didn't you call it Dumb mode rather that Deep mode. That 
>>>>>> way, I wouldn't have tried to use it and figure out what it does. ;p
>>>>>
>>>>> Target is supposed to be higher than initial! Initial == minimum 
>>>>> safe timing.
>>>>>
>>>>> You just reversed the order, try:
>>>>> sudo nvamemtiming pramin 0x6a22 5 0 -d 3
>>>>>
>>>> Ok. It's intended then. Thanks!
>>>
>>> I totally agree that this tool is a little confusing. If you have 
>>> suggestions on how to improve the usage doc, feel free to propose me 
>>> something better :)
>>>
>>> fprintf(stderr, "\t-d timing_entry_high: For each indexes, iterate 
>>> between the timing_entry and the timing_entry_high value (Deep 
>>> mode)\n");
>>>
>>> Speaking about this, this should be corrected to "For each index", 
>>> no plural after each. I'll fix it right now.
>>>
>> Ok. I will think about it.
>> Some comments in the source code can help also to know what it does 
>> and what it is used for. Same for README.
> Btw, the plural of index is indices.

Yeah, it was doubly wrong ;)

Re: [PATCH envytools] nvbios: Add missing null byte to string read from file.

[Martin Peres]

On 31/08/2014 15:00, Christian Costa wrote:
> ---
>   nvbios/nvbios.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/nvbios/nvbios.c b/nvbios/nvbios.c
> index f7aafe3..28e62ad 100644
> --- a/nvbios/nvbios.c
> +++ b/nvbios/nvbios.c
> @@ -774,11 +774,12 @@ int set_strap_from_string(const char* strap_s)
>   int set_strap_from_file(const char *path)
>   {
>   	FILE *strapfile = NULL;
> -	char tmp[21];
> +	char tmp[22];
>   
>   	strapfile = fopen(path, "r");
>   	if (strapfile) {
> -		fread(tmp, 1, 21, strapfile);
> +		size_t size = fread(tmp, 1, 21, strapfile);
> +		tmp[size] = 0;
>   		return set_strap_from_string(tmp);
>   	}
>   
Thanks, I pushed the first 3 patches!